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

[Erik V. <eri...@hc...>]

> 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.
 
(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. 

> 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.

 
> 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.