Re: [Rails-devel] Starting on a client/server

[Brett L. <wak...@gm...>]

On Tue, 2008-11-04 at 21:46 +0100, Erik Vos wrote:
> > It sounds like you feel strongly that I should bring in my changes now
> > rather than wait until they are further along. Is that about 
> > the size of
> > it?
> 
> No, that is not what I meant to say. My intention was to emphasize that,
> once you are going to check it in, that during the integration period it
> could better be kept separate in some way or other from the existing code,
> until we agree that the integration is going to be successful. But I don't
> know what your plan is - first doing an (almost) full integration
> separately?
> The point is, that I suspect that not that many classes will escape from
> being affected by your new approach.
>  

Agreed.  I had assumed that I'd just create a new CVS module, or check
it into sourceforge's SVN service if we wanted to change over to
subversion (not trying to start that discussion now, just mentioning it
because it's an available option).


> (snipped a lot)
> 
> > In general, I'd like to improve the accessor methods so that we don't
> > require GameManager to know or do as much.
> > 
> > For example, in Tile there's the isLayableNow() method:
> > 
> > public boolean isLayableNow() {
> > 	return
> > GameManager.getInstance().getCurrentPhase().isTileColourAllowe
> > d(colourName);
> > }
> 
> Good example. 
> 
> > This method is currently used exactly once in our entire 
> > codebase. This
> > method is used in OperatingRound.
> > 
> > In my mind, there are two problems with this. 
> > 
> > 1. Why do we need a whole new method in Tile for information that Tile
> > doesn't even have?
> 
> Fully agree. This logic should be moved to OperatingRound (which BTW already
> has a GameManager reference).
> 
> > 2. I think that this is information that Tile *should* have. Tile
> > shouldn't need to call all the way up to the GameManager to 
> > figure this
> > out. 
> 
> Fully disagree. Tiles should mind their own business, which is how they look
> like.
> Even the fact that they know where they are laid is somewhat suspicious; in
> fact this knowledge is only used to calculate the number of available tiles;
> so that ArrayList<MapHex> could as well be replaced by a number. 
> 

Fair enough.  I think we agree on the important point, which is that we
need to have a clear purpose for each class, and refactor anything that
has been added that doesn't line up with that purpose.

> > One likely answer is to change isLayableNow()'s signature to be
> > 'isLayableNow(phaseNumber)' or something similar. That way 
> > Tile has the
> > information it needs to be authoritative for this information, rather
> > than relying on GameManager to know this. 
> 
> As will be clear now, my solution is to replace tile.isLayableNow() (in
> OperatingRound) by currentPhase.isTileColourAllowed(tile.getColourName()).
> Then we can remove this whole silly and redundant method
> (of my own making, I'm afraid).
> 
> > My view is that Tile should be constructed with enough information to
> > know these sorts of things on its own. If it needs GameManager, it's
> > because we aren't passing in enough information to the Tile objects at
> > construction time. 
> > 
> > To fix our overuse of static methods, I think we need to improve the
> > accessor methods and how we're constructing our objects.
> > 
> > When we no longer require instances of GameManager everywhere, then we
> > no longer require GameManager to have the static 
> > getInstance() method. 
> > 
> > I believe the same logic applies to most of the other static 
> > methods we
> > have.
> > 
> > I hope that makes sense.
>  
> It sure makes, but I would phrase the solution as making a better
> distinction between game logic (I had almost written: business logic) and
> static objects. Tiles, companies, shares, trains are mainly static objects
> (although most of these must know their owners). The game logic should be in
> (or moved to) the Round instances and some related classes. That is why I
> find your example so instructive.
> 
> But not all cases will not be so easy to fix, I'm afraid.
> 

Of course, not everything is a one-line method. I picked an obvious
example to help illustrate my idea. ;-)

I think we agree on the basic concept. We need a series of objects that
hold the game state, and a series of objects that operate upon that
state, and we need to clearly define which is which.

>  
> > I think what's important is that perhaps I'm wrong about 
> > wanting to wait
> > to commit this. Perhaps what I've got so far *is* ready for a wider
> > audience and really should be committed into CVS. I was beginning to
> > second-guess myself about wanting to wait longer before I committed it
> > to the project. So, this thread was intended to get your 
> > opinions about
> > it. I figure, if I'm not sure I'm doing it right, I should run it by
> > someone else. :-)
> 
> Committing is OK with me, but please do it in a way that all the old code
> keeps running until your new code is sufficiently well-integrated to show
> its superiority....
> 
> Erik.

Alright. I think, for now, I'll just keep it in my git tree. I want to
integrate the client/server bits a little more so that when it's
committed, it'll be a stable base for integrating the rest of the
existing code.


---Brett.

Experience, n.:
	Something you don't get until just after you need it.
		-- Olivier