Mentoring session 18 Dec 2011 Sherin and Eric

[09:58 AM] csherin:	 ericgj: hello	
[09:59 AM] ericgj:	 hi csherin	
[10:01 AM] ericgj:	 give me 5 min?	
[10:01 AM] csherin:	 sure, i'll be here.	
[10:11 AM] ericgj:	 hi sorry about that	
[10:12 AM] csherin:	 np, where do we begin ?	
[10:13 AM] ericgj:	 well, I was going to ask you that.	
[10:13 AM] ericgj:	 :)	
[10:13 AM] ericgj:	 which did you feel could be improved if you had more time?	
[10:14 AM] csherin:	 i'd like a review of the chess validator problem	
[10:14 AM] csherin:	 i don't think i did a good job of it	
[10:14 AM] ericgj:	 how so?	
[10:14 AM] ericgj:	 It is definitely one of the clearest solutions I've seen	
[10:15 AM] csherin:	 for one the geometry of the board is coupled in tightly in all of the classes	
[10:16 AM] csherin:	 and i wasn't sure if the movement enumerables(Square module) had to be a part of board or the pieces	
[10:16 AM] ericgj:	 I noticed this in King#surrounding_neighbours, is that what you mean?	
[10:17 AM] csherin:	 yes that and Knight#neighbours	
[10:19 AM] csherin:	 overall it's a feeling	
[10:20 AM] ericgj:	 why does the square module need each_square_with_index?\	
[10:22 AM] csherin:	 i needed a way to load the textual representation of the board, which i chose to represent as a list	
[10:22 AM] csherin:	 so this was written to help me do that in Board#map_board	
[10:23 AM] ericgj:	 ah that explains why you `include Square`	
[10:23 AM] csherin:	 how the module Square happened is like this	
[10:24 AM] csherin:	 the enumerables were all originally within Board	
[10:24 AM] csherin:	 and this group of methods felt like a good thing to extract out	
[10:24 AM] ericgj:	 right, now I'm understanding better - Square doesn't have any state	
[10:25 AM] csherin:	 the name Square looks like a misnomer	
[10:25 AM] csherin:	 written a couple of weeks back, i'm having a hard time figuring out what i meant :D	
[10:26 AM] ericgj:	 haha I know that	
[10:27 AM] ericgj:	 so the square that you initialize the piece with seems to be the notation string?	
[10:28 AM] csherin:	 yes each square on the board, the algebraic notation(input) is the key and the piece(Chess::Piece or nil) is the value	
[10:28 AM] ericgj:	 that was what was throwing me, you had square.match and I thought, where is match in the Square class?	
[10:28 AM] ericgj:	 but Square is a module	
[10:29 AM] csherin:	 yes misleading, square is a string	
[10:29 AM] ericgj:	 besides the naming issue, getting back to your question,	
[10:30 AM] ericgj:	 where would you put the 'neighbors' geometry calculations if you were refactoring it?	
[10:31 AM] ericgj:	 I agree it doesn't seem to belong in individual piece classes	
[10:31 AM] ericgj:	 although they each have to define it specially	
[10:32 AM] csherin:	 i was thinking along the lines of the board understands it's geometry and the pieces specialize on moves. So the pieces asks the board for neighbours w.r.t its current square.	
[10:32 AM] ericgj:	 well put	
[10:34 AM] csherin:	 i'm not sure if the board geometry should be separate a class or module.	
[10:34 AM] csherin:	 it seems part of the board itself.	
[10:35 AM] ericgj:	 I would start with keeping it in the board, personally, then split it out if needed	
[10:35 AM] ericgj:	 but it still seems like the piece subclasses need to tell the board what 'a neighbor' means for them	
[10:36 AM] ericgj:	 just not in terms of board coordinates	
[10:37 AM] csherin:	 as in if a white pawn was sitting on the second rank and third column(c2)	
[10:38 AM] csherin:	 to specify the set of moves generically like	
[10:38 AM] csherin:	 [ move-up-once, move-diagonally-right-if-opponent, move-diagonally-left-if-opponent ]	
[10:38 AM] csherin:	 and so on ?	
[10:38 AM] csherin:	 instead of	
[10:39 AM] csherin:	 taking apart the algebraic notation and incrementing it...	
[10:41 AM] ericgj:	 well seeing how you could genericize the Knight#neighbors and King#surrounding_neighbors	
[10:42 AM] ericgj:	 maybe pass the board the x_offset and y_offset ?	
[10:44 AM] csherin:	 Yes. This would also separate the algebraic notation from the code.	
[10:44 AM] csherin:	 Am i following correctly ?	
[10:45 AM] ericgj:	 board.neighbors_at( (-1..1), (-1..1) ) for the King	
[10:46 AM] csherin:	 yep, generic enough	
[10:46 AM] csherin:	 can be applied for any piece	
[10:47 AM] ericgj:	 I guess you'd have to do a union for the Knight ?	
[10:47 AM] ericgj:	 i.e. union of two neighbors_at calls	
[10:48 AM] ericgj:	 (not sure *I'm* following the code right, not having worked on the problem it's kinda abstract to me !)	
[10:49 AM] csherin:	 board.neighbours_at( (-2, 2), (-1, 1) ) for the Knight	
[10:49 AM] ericgj:	 does this make any sense?	
[10:50 AM] csherin:	 notice though the x_offset and y_offset are not ranges like for the King	
[10:50 AM] ericgj:	 yeah	
[10:50 AM] csherin:	 yep, i like neighbours_at	
[10:51 AM] ericgj:	 one question I had	
[10:51 AM] ericgj:	 where do you use Board.dup ?	
[10:51 AM] ericgj:	 sorry #dup ?	
[10:52 AM] csherin:	 So when a move is made, it could also be illegal, if the move placed it's own King in check	
[10:52 AM] csherin:	 so i thought, make the move and see what happens	
[10:52 AM] csherin:	 board.dup.move(src, dst)	
[10:53 AM] csherin:	 and check that is in_check?	
[10:53 AM] csherin:	 bin/chess_validator line#32	
[10:53 AM] ericgj:	 ah, sorry I missed looking at that!	
[10:53 AM] csherin:	 it copies the entire board to check if it's in check	
[10:54 AM] csherin:	 maybe the alternative would be check it on the same board and just rollback the move	
[10:54 AM] csherin:	 maybe that's cheaper in terms of computing resources	
[10:54 AM] ericgj:	 I had to do something similar in another problem I worked on	
[10:55 AM] ericgj:	 Greg gave me feedback that 'you shouldn't override dup'	
[10:55 AM] ericgj:	 instead, use 'initialize_copy'	
[10:55 AM] ericgj:	 I never was quite sure the reasoning	
[10:56 AM] ericgj:	 but it's something maybe to ask him about	
[10:56 AM] csherin:	 The doc says, "This method may have class-specific behavior. If so, that behavior will be documented under the #initialize_copy method of the class..."	
[10:57 AM] csherin:	http://ruby-doc.org/core-1.9.3/Object.html#method-i-dup	
[10:57 AM] csherin:	 Maybe that's why	
[10:57 AM] ericgj:	 but actually, do you even need to define dup?	
[10:57 AM] ericgj:	 here, I mean doesn't Object.dup give you what you need?	
[10:58 AM] ericgj:	 sorry, scratch that, I see, you need a deep copy	
[10:58 AM] csherin:	 yes	
[10:58 AM] csherin:	 square_notation => either instance of Chess::Piece or nil	
[10:59 AM] csherin:	 but this can be done without dup, if i do the move and rollback if it happens to be a check ?	
[10:59 AM] csherin:	 but that code would instead go in the application bin/chess_validator.rb instead of the lib	
[10:59 AM] csherin:	 does that make sense	
[11:00 AM] csherin:	 or make validation of moves part of Board itself...	
[11:00 AM] ericgj:	 yes, like	
[11:02 AM] ericgj:	 neighbors.select do |notation| !board.puts_in_check?(notation) && board[notation].nil? end	
[11:02 AM] ericgj:	 or something	
[11:02 AM] ericgj:	 for #moves	
[11:03 AM] csherin:	 nice	
[11:03 AM] ericgj:	 though I don't know if that's really an improvement over how you've done it	
[11:04 AM] csherin:	 anything else which stood out for you needing improvement	
[11:05 AM] ericgj:	 well, I guess related to this,	
[11:06 AM] ericgj:	 king(colour).in_check? seems very clear, but	
[11:06 AM] ericgj:	 somehow made me nervous since `in_check?` is not a method other pieces have	
[11:07 AM] ericgj:	 maybe it's just remnants of worry from dealing with static typed languages though	
[11:08 AM] csherin:	 one worry constantly at the back of my head seems to be that board seems to be doing a lot of things	
[11:09 AM] ericgj:	 I think it's ok.	
[11:09 AM] ericgj:	 The more I work on these game simulation type problems, the more I see the board does take on a lot of the logic	
[11:10 AM] csherin:	 initially the in_check? was written entirely within board, before delegating it out to King	
[11:10 AM] csherin:	 ok	
[11:11 AM] ericgj:	 you have state changes that depend on lots of other state of other objects, it's gonna be delegated to the board (or some controller object) one way or another	
[11:12 AM] ericgj:	 so to clean up the board you often end up modularizing out the parsing, display, etc.	
[11:12 AM] csherin:	 is there a good reference implementation maybe i could read	
[11:13 AM] ericgj:	 that's a good question	
[11:13 AM] ericgj:	 there has been some talk about doing that.	
[11:14 AM] ericgj:	 But honestly, your solution, whatever its shortcomings, could be a good reference	
[11:14 AM] ericgj:	 :)	
[11:14 AM] ericgj:	 It's clean, well-documented, idiomatic code	
[11:14 AM] ericgj:	 with tests no less	
[11:14 AM] csherin:	 i'm pretty new to writing object oriented code, until recently writing everything procedurally	
[11:14 AM] ericgj:	 it's really a pleasure to read!	
[11:15 AM] csherin:	 all that is copied from reading lots of other peoples code	
[11:15 AM] ericgj:	 yes, that's really the best way isn't it?	
[11:15 AM] ericgj:	 not copied as in "copy and pasted"	
[11:15 AM] csherin:	 sure thing. amazing what you can learn.	
[11:15 AM] csherin:	 nope not copy paste :D	
[11:15 AM] ericgj:	 yes, like just reading your code I picked up some new uses for tap	
[11:16 AM] csherin:	 i meant having inline documentation, tests, classes, enumerables, etc ...	
[11:16 AM] ericgj:	 I'm going to use {}.tap instead of inject now whenever I can	
[11:17 AM] csherin:	 :)	
[11:18 AM] csherin:	 one of my goals for RMU is to better my OOP thinking and writing better Ruby	
[11:18 AM] ericgj:	 it will definitely help with that	
[11:19 AM] csherin:	 until now with a legacy code usually change becomes really difficult due to lack of or brittle design	
[11:19 AM] csherin:	 and change become really difficult as the size grew	
[11:19 AM] csherin:	 good design should probably help with that	
[11:20 AM] ericgj:	 it's always good to have motivation like that :)	
[11:21 AM] ericgj:	 I find it's a constant struggle	
[11:21 AM] ericgj:	 my production code becomes legacy like in 2 days if I don't keep refactoring it	
[11:23 AM] csherin:	 btw, thank you for both your time and help :)	
[11:23 AM] ericgj:	 np. any questions about the course?	
[11:26 AM] csherin:	 I haven't yet decided on the student project. Is there some _do_not_s when picking one.	
[11:28 AM] ericgj:	 well, not something you've worked on before the course starts	
[11:28 AM] ericgj:	 unless it's like a major new feature or something that could be considered a project in its own right	
[11:30 AM] ericgj:	 I think projects we encourage are generally - something practical, something you're motivated to do	
[11:30 AM] ericgj:	 something you can make meaningful progress on in 3 weeks	
[11:31 AM] ericgj:	 if it's a bigger project, then some reasonable chunk of it	
[11:32 AM] ericgj:	 you can see a list of others' projects here: http://university.rubymendicant.com/resources/student_projects.html	
[11:33 AM] csherin:	 Yes, i've been through those.	
[11:33 AM] ericgj:	 btw, the course runs Jan 9 - 31. Someone else was asking about that, and I couldn't find a public announcement of it, so thought I'd make sure you have the right dates	
[11:34 AM] csherin:	 I noticed not everyone makes it through the course. It takes a bit effort to apply, but why do they not complete it ?	
[11:35 AM] ericgj:	 Often it's the time committment: 10 - 20hrs a week (and often on the upper end of that) is typical	
[11:36 AM] ericgj:	 It's not a walk in the park	
[11:37 AM] csherin:	 makes sense	
[11:37 AM] csherin:	 nothing else. thank you eric	
[11:39 AM] ericgj:	 ok, np. Feel free to email/chat me if you want another session or just want to run some code by me	
[11:39 AM] ericgj:	 good luck with the course, I'm sure you'll get a lot out of it.	
[11:39 AM] csherin:	 :)	
[11:39 AM] ericgj:	 and happy holidays....	
[11:40 AM] csherin:	 same to you.