Re: [Rails-devel] Another Bug I have found, and worked out a fix for

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

1. Adding constructors with a GameManager parameter is pointless, as all
Start/Stock/OperatingRound instances are (or should be) created with
newInstance(), which uses the no-parameter constructors. That is why Round
has a setGameManager() method, to be called after creating of any round
(mostly from GameManager). Via GameManager this method (indirectly) imports
references to all static objects into the rounds (and those objects that
aren't included yet will be in the future). 

The reason I add the constructors with the GameManager parameter is to allow
the sub-class constructors, like 'ShareSellingRound' (it already had a
constructor with parameters) can call the super-class constructor with the
GameManager parameter and let IT worry about doing the initialization. Yes,
they could call setGameManager, (as you point out below). But the purpose of
a constructor is to INITIALIZE Stuff. That is their ONLY Purpose. If you
hate constructors, which it appears you do, then you can toss my
constructors, and call the appropriate initialization routines.
 


2. The special classes like ShareSellingRound are currently created in a
different way (that may change in the future, should we ever need
game-specific versions). These classes can equally well call
setGameManager(gameManager), rather that super(gameManager), making these
one-parameter constructors completely unncecessary. 

Indeed constructors are to initialize stuff, and I have nothing against
that, but you seem to turn it around by implying that initializations should
always (if anyhow possible) be done inside constructors. You seem to have
missed the point that I was trying to make above, that many classes can only
be initialized after instantiation (by a method like setGameManager) because
these classes are dynamically instantiated, which requires a constructor
that does not take any parameters (and therefore cannot initialize any
non-default values). 
 
See how most Round classes are instantiated in GameManager.createRound().
The number of classes that is created this way increases every time we find
that some type of class needs be dynamically created. So, as soon as we
discover that the choice of ShareSellingRound and/or TreasuryShareRound need
to become configurable, your reasoning falls apart for these classes, as
we'll then have to separate initialization from instantiation for these
classes as well.
 
So, as the architecture of this gaming system has developed over the past
years, and is still developing, your desire to add initializing (and hence
parametrized) constructors is a step backwards into a dead end.
 
This is my main point, and (thinking again about it) it's not a matter of
taste. The rest is.

3. In Round(), setGameManager(null) is a no-op, why adding it?


I added the call to setGameManager (null) in Round to allow for any other
initialization that setGameManager may want to do in the future, to keep the
initialization in one place. The current effect of calling setGameManager
with null changes nothing as the routine is currently written. But I hate to
NOT initialize stuff, nor do I like to use default values. If I have code
specifically to set the value to something, I know where it is set. But
again, this is style/taste. I can take it or leave it really.  

 Indeed. I prefer shorthand, you prefer to spell things out. Doesn't really
matter.

4. Again probably a matter of taste. The following three statements are all
equivalent:
 
    StockRound() { super(); }
    StockRound() {}
    <empty>
 
because the compiler is so friendly to add all that is missing. In such
matters I tend to code minimally: leaving out what is automatic or obvious
(in pedantic mode I like to say that I apply Occam's razor; I admit that
existing empty constructors like OperatingRound() had escaped that razor so
far). You seem more inclined to code maximally here, including stuff that
will exist anyway (stating the obvious, as it were). I tend to take "cleaner
code" pretty literally: "less code".



This is again some of my old-school programming training. Don't trust the
compiler to do this sort of work for you. If you specifically state it, and
be clear about it, the compile won't mess it up. By leaving it "empty" you
make an assumption. And the bug I found was based on your assumption that
the initialization always worked. and yet a step was missed.

Java guarantees this compiler behaviour, I see no reason not to trust that.
But if you want these (parameterless) constructors, fine with me.
 
I skip the remaining (minor) points from the previous posts.
 
Erik.