rails-devel

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

[Mark S. <mar...@gm...>]

I found a reasonable quick way in 1830 to force a player to buy a train out
of hand (3 player game, start B&O, PRR, and a 3rd Railroad all $67 per
share. B&O buys four (4) two trains (yes stupid plan, but this is to test
the code), PRR buy four trains, the 3rd Railroad buy the last three trains.
Next Stock Round start company #4 at $67 per share, and then in the next OR,
have that company buy a 4 train - killing the twos before B&O can operate.
Now when B&O operates, no trains, and < $300 in company and hand together
(B&O also needs to build $160 (two tiles), and buy a Token.

Then... we get to a ShareSellingRound launching.

And it goes BOOM!  --- Null Pointer Exception right at the start of
setSellableShares for loop, because companyManager is not set in the
constructor.

Digging into this class structure if find the following:

+ -- Round
      |
      +-- StockRound extends Round
      |    |
      |    +-- ShareSellingRound extends StockRound
      |    |
      |    +-- TreasuryShareRound extends StockRound
      |    |
      |    +-- StockRound_1856 extends StockRound
      |    |
      |    +-- StockRound_1835 extends StockRound
      |    |
      |    +-- StockRound_18EU extends StockRound
      |
      +-- OperatingRound extends Round
      |    |
      |    +-- OperatingRound_1856 extends OperatingRound
      |    |
      |    +-- OperatingRound_18EU extends OperatingRound
      |    |
      |    +-- OperatingRound_18AL extends OperatingRound
      |
      +-- StartRound extends OperatingRound
           |
           +-- StartRound_1830 extends StartRound
           |
           +-- StartRound_1835 extends StartRound
           |
           +-- StartRound_1851 extends StartRound
           |
           +-- StartRound_18EU extends StartRound

Issues that I see:
1. The StartRound_1830.java and StartRound_1835.java should be placed under
the specific/_1830 and specific/_1835 sub-folders along with the other
special case classes
2. The Round, StockRound, OperatingRound, and the respective subclasses do
not have constructors for no elements, or for theStockRound (and
sub-classes) an cosntructor with the GameManager  Round superclass should
have (but doesn't).  The Round superclass does have a method
'setGameManager' which assigns gameManager and companyManager.

I have adjusted my copies of the various classes to resolve this Null
Pointer Exception, by creating constructors for the Round, StockRound,
OperatingRound and the sub-classes of StockRound. It does require
modifications of all of these for the constructors. A total of 8 classes to
update. (the sub-classes of OperatingRound does not require an update. But
probably should have constructors created for consistancy's sake.

Since I am creating new methods, should I also add in the JavaDocs for them
as well?

Mark

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

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

>
> Issues that I see:
> 1. The StartRound_1830.java and StartRound_1835.java should be placed
> under the specific/_1830 and specific/_1835 sub-folders along with the
> other special case classes

Yes and no.
These classes pre-date the introduction of the game.specific package, and
are in fact not really game-specific. The 1830-style auction occurs in
many games, and I believe the 1835-style auction is also used in 1837.
At the time these classes were created I had a brief discussion with Brett
about the naming of such classes, but we couldn't find good generic names
for the different auction types. Perhaps StartRound1830Style would have
been better.

I have no real objection against moving these classes to game.specific,
but the above should be kept in mind.

> 2. The Round, StockRound, OperatingRound, and
> the respective subclasses do not have constructors for no elements, or for
> theStockRound (and sub-classes) an cosntructor with the GameManager  Round
> superclass should have (but doesn't).  The Round superclass does have a
> method 'setGameManager' which assigns gameManager and companyManager.
>
>
> I have adjusted my copies of the various classes to resolve this Null
> Pointer Exception, by creating constructors for the Round, StockRound,
> OperatingRound and the sub-classes of StockRound. It does require
> modifications of all of these for the constructors. A total of 8 classes
> to update. (the sub-classes of OperatingRound does not require an update.
> But
> probably should have constructors created for consistancy's sake.

I don't have the current code handy here at work (just an old version),
but I would think that a Round constructor should do all the common tasks,
and the next lower level (StockRound etc.) any additional tasks specific
to the type of round. So I'm not sure why all these classes need explicit
constructors. But I'll have a look tonight. I'm probably missing some
point.

I know that the whole initialization stuff is somewhat messy, so I am not
at all surprised that you are finding inconsistencies.

> Since I am creating new methods, should I also add in the JavaDocs for
> them as well?

Yes, of course that would be a good thing to do. I know I'm sloppy in
creating Javadocs, perhaps I can spend some time on that before the
upcoming release, if we're running out of bugs.

Erik.

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

[Mark S. <mar...@gm...>]

My first Suggestion was based upon as you guessed the "Game Specific"
structure you have in place. I have not changed my copy to reflect that.
This was purely a suggestion.

As for the second suggestion, I have indeed implemented the constructors
such that the super-classes do all the initialization that should, and the
sub-classes do the initialization they should. In a couple of places I
needed to create a constructor that simply called the super class
constructor and nothing more.

I will apply the JAVA Docs to the constructors I have created, and check
them in this evening.

I have noticed that XCode does warn about several classes marked as
deprecated (in your comments) that are still being used. But the deprecation
comment does not specify what should be used instead. Those might be a
better item to clean up then simply adding JAVA Docs.

Mark

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

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

I have adjusted my copies of the various classes to resolve this Null
Pointer Exception, by creating constructors for the Round, StockRound,
OperatingRound and the sub-classes of StockRound. It does require
modifications of all of these for the constructors. A total of 8 classes to
update. (the sub-classes of OperatingRound does not require an update. But
probably should have constructors created for consistancy's sake.

The simple fix would have been to add
    this.companyManager = gameManager.getCompanyManager();

to the ShareSellingRound constructor. That works for me to fix the problem
(which I have reproduced). Not sure what you are trying to achieve by adding
all these constructors.

As for the second suggestion, I have indeed implemented the constructors
such that the super-classes do all the initialization that should, and the
sub-classes do the initialization they should. In a couple of places I
needed to create a constructor that simply called the super class
constructor and nothing more.  

I wonder why you are using worls like "required" and "needed": such default
constructors can be omitted.

I have noticed that XCode does warn about several classes marked as
deprecated (in your comments) that are still being used. But the deprecation
comment does not specify what should be used instead. Those might be a
better item to clean up then simply adding JAVA Docs.

Good point. Thanks.
 
Erik.

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

[Mark S. <mar...@gm...>]

Yes, the simple fix is as you state. However, the purpose for sub-classing
is to use the super-class constructor routines to initialize stuff at that
superclass level, and use the sub-class constructors to call the super class
constructor, and then initialize what the sub-class needs. Without doing
proper constructors, at all levels, you wind up forgetting to initialize
something (as in this case). It may introduce some extra code in some
places, but it reduces errors like this one.

Am I not being clear?

I hate messy code, and would much prefer to follow at least a
pseudo-standard.

I was using "required" and "needed" because when I tried to create just the
super-class constructor, the compiler complained the sub-class did not have
the appropriate constructor. So I created those, and cleaned up stuff as I
saw it needed to be cleaned up.



On Tue, Dec 2, 2008 at 3:13 PM, Erik Vos <eri...@hc...> wrote:

>  I have adjusted my copies of the various classes to resolve this Null
> Pointer Exception, by creating constructors for the Round, StockRound,
> OperatingRound and the sub-classes of StockRound. It does require
> modifications of all of these for the constructors. A total of 8 classes to
> update. (the sub-classes of OperatingRound does not require an update. But
> probably should have constructors created for consistancy's sake.
>
> The simple fix would have been to add
>
> *    this*.companyManager = gameManager.getCompanyManager();
>
> to the ShareSellingRound constructor. That works for me to fix the problem
> (which I have reproduced). Not sure what you are trying to achieve by adding
> all these constructors.
>
> As for the second suggestion, I have indeed implemented the constructors
> such that the super-classes do all the initialization that should, and the
> sub-classes do the initialization they should. In a couple of places I
> needed to create a constructor that simply called the super class
> constructor and nothing more.
>
> I wonder why you are using worls like "required" and "needed": such default
> constructors can be omitted.
>
> I have noticed that XCode does warn about several classes marked as
> deprecated (in your comments) that are still being used. But the deprecation
> comment does not specify what should be used instead. Those might be a
> better item to clean up then simply adding JAVA Docs.
>
> Good point. Thanks.
>
> Erik.
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
>

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

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

Well, I guess I have to wait and see what you come up with. Your standard
may work for the Rounds, but keep in mind that not all initializations can
be done in constructors because of sequence issues, which is exactly why
there are so many empty or missing constructors in the game engine.
 
Erik.


  _____  

From: Mark Smith [mailto:mar...@gm...] 
Sent: Wednesday 03 December 2008 00:52
To: Development list for Rails: an 18xx game
Subject: Re: [Rails-devel] Another Bug I have found, and worked out a fix
for


Yes, the simple fix is as you state. However, the purpose for sub-classing
is to use the super-class constructor routines to initialize stuff at that
superclass level, and use the sub-class constructors to call the super class
constructor, and then initialize what the sub-class needs. Without doing
proper constructors, at all levels, you wind up forgetting to initialize
something (as in this case). It may introduce some extra code in some
places, but it reduces errors like this one. 

Am I not being clear?

I hate messy code, and would much prefer to follow at least a
pseudo-standard. 

I was using "required" and "needed" because when I tried to create just the
super-class constructor, the compiler complained the sub-class did not have
the appropriate constructor. So I created those, and cleaned up stuff as I
saw it needed to be cleaned up.




On Tue, Dec 2, 2008 at 3:13 PM, Erik Vos <eri...@hc...> wrote:


I have adjusted my copies of the various classes to resolve this Null
Pointer Exception, by creating constructors for the Round, StockRound,
OperatingRound and the sub-classes of StockRound. It does require
modifications of all of these for the constructors. A total of 8 classes to
update. (the sub-classes of OperatingRound does not require an update. But
probably should have constructors created for consistancy's sake.

The simple fix would have been to add
    this.companyManager = gameManager.getCompanyManager();

to the ShareSellingRound constructor. That works for me to fix the problem
(which I have reproduced). Not sure what you are trying to achieve by adding
all these constructors.

As for the second suggestion, I have indeed implemented the constructors
such that the super-classes do all the initialization that should, and the
sub-classes do the initialization they should. In a couple of places I
needed to create a constructor that simply called the super class
constructor and nothing more.  

I wonder why you are using worls like "required" and "needed": such default
constructors can be omitted.

I have noticed that XCode does warn about several classes marked as
deprecated (in your comments) that are still being used. But the deprecation
comment does not specify what should be used instead. Those might be a
better item to clean up then simply adding JAVA Docs.

Good point. Thanks.
 
Erik.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great
prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100
<http://moblin-contest.org/redirect.php?banner_id=100&url=/> &url=/
_______________________________________________
Rails-devel mailing list
Rai...@li...
https://lists.sourceforge.net/lists/listinfo/rails-devel

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

[Mark S. <mar...@gm...>]

OK. I can accept that.

What I find disturbing is initializing something in the super class by the
sub-class, when it is NOT a special case initialization for the sub-class. I
will get the stuff checked in this evening. With proper Java Docs.

Mark

On Wed, Dec 3, 2008 at 3:32 PM, Erik Vos <eri...@hc...> wrote:

>  Well, I guess I have to wait and see what you come up with. Your standard
> may work for the Rounds, but keep in mind that not all initializations can
> be done in constructors because of sequence issues, which is exactly why
> there are so many empty or missing constructors in the game engine.
>
> Erik.
>
>  ------------------------------
> *From:* Mark Smith [mailto:mar...@gm...]
> *Sent:* Wednesday 03 December 2008 00:52
> *To:* Development list for Rails: an 18xx game
> *Subject:* Re: [Rails-devel] Another Bug I have found, and worked out a
> fix for
>
> Yes, the simple fix is as you state. However, the purpose for sub-classing
> is to use the super-class constructor routines to initialize stuff at that
> superclass level, and use the sub-class constructors to call the super class
> constructor, and then initialize what the sub-class needs. Without doing
> proper constructors, at all levels, you wind up forgetting to initialize
> something (as in this case). It may introduce some extra code in some
> places, but it reduces errors like this one.
>
> Am I not being clear?
>
> I hate messy code, and would much prefer to follow at least a
> pseudo-standard.
>
> I was using "required" and "needed" because when I tried to create just the
> super-class constructor, the compiler complained the sub-class did not have
> the appropriate constructor. So I created those, and cleaned up stuff as I
> saw it needed to be cleaned up.
>
>
>
> On Tue, Dec 2, 2008 at 3:13 PM, Erik Vos <eri...@hc...> wrote:
>
>>   I have adjusted my copies of the various classes to resolve this Null
>> Pointer Exception, by creating constructors for the Round, StockRound,
>> OperatingRound and the sub-classes of StockRound. It does require
>> modifications of all of these for the constructors. A total of 8 classes to
>> update. (the sub-classes of OperatingRound does not require an update. But
>> probably should have constructors created for consistancy's sake.
>>
>> The simple fix would have been to add
>>
>> *    this*.companyManager = gameManager.getCompanyManager();
>>
>> to the ShareSellingRound constructor. That works for me to fix the problem
>> (which I have reproduced). Not sure what you are trying to achieve by adding
>> all these constructors.
>>
>> As for the second suggestion, I have indeed implemented the constructors
>> such that the super-classes do all the initialization that should, and the
>> sub-classes do the initialization they should. In a couple of places I
>> needed to create a constructor that simply called the super class
>> constructor and nothing more.
>>
>> I wonder why you are using worls like "required" and "needed": such
>> default constructors can be omitted.
>>
>> I have noticed that XCode does warn about several classes marked as
>> deprecated (in your comments) that are still being used. But the deprecation
>> comment does not specify what should be used instead. Those might be a
>> better item to clean up then simply adding JAVA Docs.
>>
>> Good point. Thanks.
>>
>> Erik.
>>
>> -------------------------------------------------------------------------
>> This SF.Net email is sponsored by the Moblin Your Move Developer's
>> challenge
>> Build the coolest Linux based applications with Moblin SDK & win great
>> prizes
>> Grand prize is a trip for two to an Open Source event anywhere in the
>> world
>> http://moblin-contest.org/redirect.php?banner_id=100&url=/
>> _______________________________________________
>> Rails-devel mailing list
>> Rai...@li...
>> https://lists.sourceforge.net/lists/listinfo/rails-devel
>>
>>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
>

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

[Mark S. <mar...@gm...>]

Alright, due to the oddity of XCode, that I still have not got completely
figured out, I have gotten the nine (9) classes updated and committed with
the new constructors. I had to check them in one at a time and fiddle with
CVS Entries. Dang annoying.

I honestly really like XCode for the development side, but it has very
strange behavior when interacting with CVS on source forge... or I am doing
something really weird that I can't tell.

Feel free to look over my constructors...and let me know if I have mucked up
something or actually made the code cleaner please.

Mark

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

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

      Feel free to look over my constructors...and let me know if I have
mucked up something or actually made the code cleaner please.

OK, here we go. Much of what I'm going to say is probably a matter of taste
(or style), and our tastes apparently differ. Nothing wrong with that. But
I, for me, don't see much improvement.
 
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). 
 
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.
 
3. In Round(), setGameManager(null) is a no-op, why adding it?
 
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".
 
5. I personally hate these aThis and theThat variable names; YMMV.
 
6. I'm no fan of Javadoc comments like "Constructor with no parameters, call
the super Class (Round's) Constructor with no parameters". Javadocs should
describe a method's purpose or usage rather than its contents.
 
On the positive side, your version of setGameManager() is better, and it has
also become clear that ShareSellingRound and TreasuryShareRound should call
this method rather than doing their own settings.
 
I hope I haven't disappointed you too much - it's not my intention to put
you off, but I do think that you have added unnecessary code.
 
Erik.
 
 
 

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

[Mark S. <mar...@gm...>]

On Thu, Dec 4, 2008 at 3:54 PM, Erik Vos <eri...@hc...> wrote:

>
>       Feel free to look over my constructors...and let me know if I have
> mucked up something or actually made the code cleaner please.
>
> OK, here we go. Much of what I'm going to say is probably a matter of taste
> (or style), and our tastes apparently differ. Nothing wrong with that. But
> I, for me, don't see much improvement.
>
>

Style/Taste of course is a large part of why I was tinkering here. And yes,
different people have different preferences


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


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


>
>  5. I personally hate these aThis and theThat variable names; YMMV.
>

Here is one of my purely personaly taste items. I tend to write code with
the arguments comming in prefixed with 'a' and local method variable
prefixed with 't' to make it clear within the code where it came from. If
this is something that will get under your skin, then before I add my code
base, I will need to go through it and strip those prefixes out.


>
> 6. I'm no fan of Javadoc comments like "Constructor with no parameters,
> call the super Class (Round's) Constructor with no parameters". Javadocs
> should describe a method's purpose or usage rather than its contents.
>

Since this was my first true attempt to write Javadocs, I was taking a
guess. I will be flexible here, and see if I can do better in the future.


>
> On the positive side, your version of setGameManager() is better, and it
> has also become clear that ShareSellingRound and TreasuryShareRound should
> call this method rather than doing their own settings.
>
>

At least I did something that you don't outright hate. :-) But the
setGameManager I did not alter in any way (the one in Round.java). I simply
had the constructors call it properly.

It all gets back to my initial point of which level of the class hierarchy
should initialize what parts.

I hope I haven't disappointed you too much - it's not my intention to put
> you off, but I do think that you have added unnecessary code.
>
>

Disappointed? - No. as you stated, it is mostly Style/Taste. You and Brett
have been engineering most of this product so far, so my comming in needs to
be balanced with your coding style. I cannot be the maverick who comes in
and shoots holes in everything everywhere just because I carry a shotgun,
and think I know how to use it.

I do feel I have much to contribute still.

But be warned, I will be picking on other aspects of the implementation,
here and there. There is some items that I have seen that set my teeth on
edge... (for example 'counting the number of passes' to determine if a stock
round is over)


> Erik.
>
>
>
>
>
> ------------------------------------------------------------------------------
> SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
> The future of the web can't happen without you.  Join us at MIX09 to help
> pave the way to the Next Web now. Learn more and register at
>
> http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
>

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.
 

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

[Mark S. <mar...@gm...>]

OK, if you feel that parametrized constructors is a step backwards, I will
comply with your request. I don't agree, but I won't argue about it any
more.

Mark

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

[Jean M. (Europe) <Jea...@eu...>]

Hi Mark, Erik, everyone,
 
Just a small note to say I have followed the discussion, and without any
pretention to say I understand the gritty details, Erik says that the
classes in question are being instantiated "dynamically", e.g. in a way
such that constructors would be inappropriate.  So he says he would be
happier to leave the empty constructors and use the setGameManager() or
any other configuration method.
 
Erik, would you be so kind as pointing out very pragmatically to one
specific such case?  I think it might enlighten at least myself on the
way the architecture has been thought out and built.  This would have
all of us looking into the same direction (or question the direction,
but this would certainly not come from me).
 
Thank you,
 
Jean (John in French) Michalski


________________________________

	From: Mark Smith [mailto:mar...@gm...] 
	Sent: 05 December 2008 23:43
	To: Development list for Rails: an 18xx game
	Subject: Re: [Rails-devel] Another Bug I have found, and worked
out a fix for
	
	
	OK, if you feel that parametrized constructors is a step
backwards, I will comply with your request. I don't agree, but I won't
argue about it any more.
	
	Mark
	

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

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

Hi Jean,
 
The point is, that several class names are configurable. For instance, the
default operating round class is OperatingRound, but several games use more
specialized subclasses, with names like OperatingRound_18xx. Examples are
18AL and 18EU; see the <OperatingRound> tags in the Games.xml files of these
games.
 
These classes (and most other Round subclasses) are dynamically instantiated
in method createRound() of class GameManager, by calling the newInstance()
method of a Class object for the particular class to be created. The
newInstance() method requires a parameterless constructor, like
OperatingRound(). So we can only use such constructors; constructors that
take parameters are useless for these classes. 
 
(I think there are ways to create newInstance methods that take parameters
as well, but I didn't find it worth while to sort out how to jump through
those hoops, as we already had a simple solution).
 
Parameterless constructors can obviously only initialize fixed/default
values, nothing that is variable (except through calling class methods, but
we want to get away from class methods). That means that all further
initialization must be done by calling a non-constructor method; in this
case setGameManager().
 
I hope this helps.
Erik.


  _____  

From: Jean Michalski (Europe) [mailto:Jea...@eu...] 
Sent: Sunday 07 December 2008 09:46
To: Development list for Rails: an 18xx game
Subject: Re: [Rails-devel] Another Bug I have found, and worked out a fix
for


Hi Mark, Erik, everyone,
 
Just a small note to say I have followed the discussion, and without any
pretention to say I understand the gritty details, Erik says that the
classes in question are being instantiated "dynamically", e.g. in a way such
that constructors would be inappropriate.  So he says he would be happier to
leave the empty constructors and use the setGameManager() or any other
configuration method.
 
Erik, would you be so kind as pointing out very pragmatically to one
specific such case?  I think it might enlighten at least myself on the way
the architecture has been thought out and built.  This would have all of us
looking into the same direction (or question the direction, but this would
certainly not come from me).
 
Thank you,
 
Jean (John in French) Michalski


  _____  

From: Mark Smith [mailto:mar...@gm...] 
Sent: 05 December 2008 23:43
To: Development list for Rails: an 18xx game
Subject: Re: [Rails-devel] Another Bug I have found, and worked out a fix
for


OK, if you feel that parametrized constructors is a step backwards, I will
comply with your request. I don't agree, but I won't argue about it any
more.

Mark

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

[Jean M. (Europe) <Jea...@eu...>]

Hi Erik,
 
Thank you for the answer, it becomes more clear.  So in short, it is an
implementation choice of yours to use java.lang.Class.newInstance()
instead of java.lang.reflect.Constructor.newInstance(Object[] initargs).
 
Kind regards, and best wishes in your endeavours,
 
Jean.


________________________________

	From: Erik Vos [mailto:eri...@hc...] 
	Sent: 08 December 2008 00:22
	To: 'Development list for Rails: an 18xx game'
	Subject: Re: [Rails-devel] Another Bug I have found, and worked
out a fix for
	
	
	Hi Jean,
	 
	The point is, that several class names are configurable. For
instance, the default operating round class is OperatingRound, but
several games use more specialized subclasses, with names like
OperatingRound_18xx. Examples are 18AL and 18EU; see the
<OperatingRound> tags in the Games.xml files of these games.
	 
	These classes (and most other Round subclasses) are dynamically
instantiated in method createRound() of class GameManager, by calling
the newInstance() method of a Class object for the particular class to
be created. The newInstance() method requires a parameterless
constructor, like OperatingRound(). So we can only use such
constructors; constructors that take parameters are useless for these
classes. 
	 
	(I think there are ways to create newInstance methods that take
parameters as well, but I didn't find it worth while to sort out how to
jump through those hoops, as we already had a simple solution).
	 
	Parameterless constructors can obviously only initialize
fixed/default values, nothing that is variable (except through calling
class methods, but we want to get away from class methods). That means
that all further initialization must be done by calling a
non-constructor method; in this case setGameManager().
	 
	I hope this helps.
	Erik.


________________________________

		From: Jean Michalski (Europe)
[mailto:Jea...@eu...] 
		Sent: Sunday 07 December 2008 09:46
		To: Development list for Rails: an 18xx game
		Subject: Re: [Rails-devel] Another Bug I have found, and
worked out a fix for
		
		
		Hi Mark, Erik, everyone,
		 
		Just a small note to say I have followed the discussion,
and without any pretention to say I understand the gritty details, Erik
says that the classes in question are being instantiated "dynamically",
e.g. in a way such that constructors would be inappropriate.  So he says
he would be happier to leave the empty constructors and use the
setGameManager() or any other configuration method.
		 
		Erik, would you be so kind as pointing out very
pragmatically to one specific such case?  I think it might enlighten at
least myself on the way the architecture has been thought out and built.
This would have all of us looking into the same direction (or question
the direction, but this would certainly not come from me).
		 
		Thank you,
		 
		Jean (John in French) Michalski


________________________________

			From: Mark Smith [mailto:mar...@gm...]

			Sent: 05 December 2008 23:43
			To: Development list for Rails: an 18xx game
			Subject: Re: [Rails-devel] Another Bug I have
found, and worked out a fix for
			
			
			OK, if you feel that parametrized constructors
is a step backwards, I will comply with your request. I don't agree, but
I won't argue about it any more.
			
			Mark
			

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

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

Hi Jean,
 
I'm afraid the word "choice" is not really applicable, as I was only vaguely
aware of the second possibility. 
I have looked a bit around, but I have trouble finding a good usage example.
Do you know how to rewrite GameManager.createRound() using reflection, in
Java 5.0 style (with generics), passing gameManager to a Round subclass
constructor taking this single object?
 
If we can get this done, I'm happy to give in to Mark.
 
Thanks for your best wishes, and perhaps it'll even get better if you
contribute some of your wisdom....
 
Erik.


  _____  

From: Jean Michalski (Europe) [mailto:Jea...@eu...] 
Sent: Monday 08 December 2008 08:02
To: Development list for Rails: an 18xx game
Subject: Re: [Rails-devel] Another Bug I have found, and worked out a fix
for


Hi Erik,
 
Thank you for the answer, it becomes more clear.  So in short, it is an
implementation choice of yours to use java.lang.Class.newInstance() instead
of java.lang.reflect.Constructor.newInstance(Object[] initargs).
 
Kind regards, and best wishes in your endeavours,
 
Jean.


  _____  

From: Erik Vos [mailto:eri...@hc...] 
Sent: 08 December 2008 00:22
To: 'Development list for Rails: an 18xx game'
Subject: Re: [Rails-devel] Another Bug I have found, and worked out a fix
for


Hi Jean,
 
The point is, that several class names are configurable. For instance, the
default operating round class is OperatingRound, but several games use more
specialized subclasses, with names like OperatingRound_18xx. Examples are
18AL and 18EU; see the <OperatingRound> tags in the Games.xml files of these
games.
 
These classes (and most other Round subclasses) are dynamically instantiated
in method createRound() of class GameManager, by calling the newInstance()
method of a Class object for the particular class to be created. The
newInstance() method requires a parameterless constructor, like
OperatingRound(). So we can only use such constructors; constructors that
take parameters are useless for these classes. 
 
(I think there are ways to create newInstance methods that take parameters
as well, but I didn't find it worth while to sort out how to jump through
those hoops, as we already had a simple solution).
 
Parameterless constructors can obviously only initialize fixed/default
values, nothing that is variable (except through calling class methods, but
we want to get away from class methods). That means that all further
initialization must be done by calling a non-constructor method; in this
case setGameManager().
 
I hope this helps.
Erik.


  _____  

From: Jean Michalski (Europe) [mailto:Jea...@eu...] 
Sent: Sunday 07 December 2008 09:46
To: Development list for Rails: an 18xx game
Subject: Re: [Rails-devel] Another Bug I have found, and worked out a fix
for


Hi Mark, Erik, everyone,
 
Just a small note to say I have followed the discussion, and without any
pretention to say I understand the gritty details, Erik says that the
classes in question are being instantiated "dynamically", e.g. in a way such
that constructors would be inappropriate.  So he says he would be happier to
leave the empty constructors and use the setGameManager() or any other
configuration method.
 
Erik, would you be so kind as pointing out very pragmatically to one
specific such case?  I think it might enlighten at least myself on the way
the architecture has been thought out and built.  This would have all of us
looking into the same direction (or question the direction, but this would
certainly not come from me).
 
Thank you,
 
Jean (John in French) Michalski


  _____  

From: Mark Smith [mailto:mar...@gm...] 
Sent: 05 December 2008 23:43
To: Development list for Rails: an 18xx game
Subject: Re: [Rails-devel] Another Bug I have found, and worked out a fix
for


OK, if you feel that parametrized constructors is a step backwards, I will
comply with your request. I don't agree, but I won't argue about it any
more.

Mark

[Rails-devel] Cleanups

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

I have noticed that XCode does warn about several classes marked as
deprecated (in your comments) that are still being used. But the deprecation
comment does not specify what should be used instead. Those might be a
better item to clean up then simply adding JAVA Docs.

 
I have cleaned up most of those, but also left in one that is only used in
the external tile XML conversion utilities. Too lazy now to rework that
code.
And cleaned up more code based on the compiler warnings. The remaining
warnings are inevitable, or refer to variables for future use or to the
remaining deprecated method. 
 
Erik.

Re: [Rails-devel] Cleanups

[Mark S. <mar...@gm...>]

I still get about 30 warnings (I had 32 before)... --

getSpecialProperties () in rails.game.RoundI has been deprecated
percentageOwnedByPlayers() in rails.game.PublicCompanyI has been deprecated
getStatus() in rails.game.action.StartItemAction has been deprecated
getElement() in rails.util.Tag has been deprecated (in
ConvertTilesXML.java:167 and MakeGameTileSets.java:66,94,102)

Mostly the getSpecialProperties one.

Mark


On Tue, Dec 2, 2008 at 3:35 PM, Erik Vos <eri...@hc...> wrote:

>  I have noticed that XCode does warn about several classes marked as
> deprecated (in your comments) that are still being used. But the deprecation
> comment does not specify what should be used instead. Those might be a
> better item to clean up then simply adding JAVA Docs.
>
>
> I have cleaned up most of those, but also left in one that is only used in
> the external tile XML conversion utilities. Too lazy now to rework that
> code.
> And cleaned up more code based on the compiler warnings. The remaining
> warnings are inevitable, or refer to variables for future use or to the
> remaining deprecated method.
>
> Erik.
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
>

Re: [Rails-devel] Cleanups

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

Probably my warning settings are a bit lower than yours...

I still get about 30 warnings (I had 32 before)... --

getSpecialProperties () in rails.game.RoundI has been deprecated
percentageOwnedByPlayers() in rails.game.PublicCompanyI has been deprecated 

Removed (were unused). 

        getStatus() in rails.game.action.StartItemAction has been deprecated
 
Undeprecated (there must have been a good reason why I did it, but I don't
remember).

         getElement() in rails.util.Tag has been deprecated (in
ConvertTilesXML.java:167 and MakeGameTileSets.java:66,94,102)
 
Will stay so for a while. Your new map code may make these utilities
obsolete. I'll wait for that.
 
Erik.

 

Re: [Rails-devel] Cleanups

[Mark S. <mar...@gm...>]

OK with these cleanups, I am down to four warnings, just the getLement
items. I can accept them.

Thanks.

On Wed, Dec 3, 2008 at 3:23 PM, Erik Vos <eri...@hc...> wrote:

>  Probably my warning settings are a bit lower than yours...
>
>  I still get about 30 warnings (I had 32 before)... --
>
> getSpecialProperties () in rails.game.RoundI has been deprecated
> percentageOwnedByPlayers() in rails.game.PublicCompanyI has been deprecated
>
>
> Removed (were unused).
>
>         getStatus() in rails.game.action.StartItemAction has been
> deprecated
>
> Undeprecated (there must have been a good reason why I did it, but I don't
> remember).
>
>          getElement() in rails.util.Tag has been deprecated (in
> ConvertTilesXML.java:167 and MakeGameTileSets.java:66,94,102)
>
> Will stay so for a while. Your new map code may make these utilities
> obsolete. I'll wait for that.
>
> Erik.
>
>
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
>