From: Ben Schaffhausen <bhs128@...>
Date: 2006-09-22T03:17:18+09:00
Subject: Re: Request for code review of beginning project

Thanks for taking the time to look through it- I appreciate the advice.

I'll have to review how to write test cases again and put some in place.

I intend on integrating this file/set of classes in with a Ruby-GNOME2 / 
OpenGL interface but am kind of aprehensive about where to instatiate my 
classes and still maintain decent seperation of code.

Obviously there is going to be a lot of interaction as GTK takes in the 
user input, and through gtk/opengl commands handles the output. My 
current idea is to have all of the building(and perhaps courier) objects 
have a draw() function where it will draw it's current state. So somehow 
both my draw loop and event handlers (each in different classes already) 
both need access to all of my City classes (and eventually a Player 
class of some sort to hold multiple cities)

I'm already in a similar situation in my current test opengl program (a 
cube that I can rotate and zoom (ArcBall rotations/Quarternions) and I'm 
already using global variables for the rot_x, rot_y and zoom (so that 
both the event handlers and the opengl draw function have access).  I'll 
post that code tonight.

Anyways, I'm kinda thinking out loud at this point, any comments are 
always appreciated.

Thanks again,
Ben Schaffhausen

James Gray wrote:
> On Sep 21, 2006, at 2:55 AM, Ben Schaffhausen wrote:
> 
>> Hello,
> 
> Hello and welcome to Ruby.
> 
>> To start I am making a few classes to model the buildings and delivery
>> or materials to and from other buildings. Currently the program is  
>> ~250
>> lines long and seems to work within my intentions thus far.
> 
> "seems to work" is a good sign that you might enjoy learning a little
> about unit testing.  You could build tests to to ensure that it is
> indeed working as you expect and give you peace of mind as the code
> grows.
> 
>> Still I can't help but feel that the code is messy or that I'm doing
>> things the hard way/non-ruby-way.
> 
> I really didn't see any big red flags in the code.  Mostly you are
> assigning to or making minor adjustments to variables.  There's not a
> lot for us to improve on for something like that.
> 
> Some general tips:
> 
> *  Drop the parens around if( ..) lines.  Ruby doesn't need them.
> *  Try to use indices as little as possible.  They are generally un-
> Rubyish.  Here's an example targeting your queue:
> 
>  >> # with indices...
> ?> @q = [ ["Beads", "Denver", "Oklahoma City"],
> ?>        ["Keyboards", "New Haven", "Chicago"] ]
> => [["Beads", "Denver", "Oklahoma City"], ["Keyboards", "New Haven",
> "Chicago"]]
>  >> # show what is going to where
> ?> @q.each { |item| puts "#{item[0]} to #{item[2]}" }
> Beads to Oklahoma City
> Keyboards to Chicago
> => [["Beads", "Denver", "Oklahoma City"], ["Keyboards", "New Haven",
> "Chicago"]]
>  >> # without indices...
> ?> Shippment = Struct.new(:product, :origin, :destination)
> => Shippment
>  >> @q = [ Shippment.new("Beads", "Denver", "Oklahoma City"),
> ?>        Shippment.new("Keyboards", "New Haven", "Chicago") ]
> => [#<struct Shippment product="Beads", origin="Denver",
> destination="Oklahoma City">, #<struct Shippment product="Keyboards",
> origin="New Haven", destination="Chicago">]
>  >> # show what is going to where
> ?> @q.each { |ship| puts "#{ship.product} to #{ship.destination}" }
> Beads to Oklahoma City
> Keyboards to Chicago
> => [#<struct Shippment product="Beads", origin="Denver",
> destination="Oklahoma City">, #<struct Shippment product="Keyboards",
> origin="New Haven", destination="Chicago">]
> 
> Hope that helps.
> 
> James Edward Gray II


-- 
Posted via http://www.ruby-forum.com/.