Closed erik-vos closed 2 years ago
During 1837 development, at several occasions I met a requirement to provide identical or very similar methods in different class hierarchies. Examples: (1) methods that would be identical for 1835 and 1837, but would have to be added to game-specific classes, like StockRound_1835 and StockRound_1837 because that code would be only relevant to these two games; (2) code related to share selling, that should derive both from Stock_Round_1837 and ShareSellingRound (unfortunately, java does not support multiple inheritance); (3) code that should be used both in SR- and in OR-derived classes.
In the past, I handled such cases by adding methods to the Round superclass (which, by the way, is no longer abstract). But other people did not like that, as is shown by several comments in that class. And I can understand that.
Another approach for which I found a few examples, is to create a separate class, having static methods only, so that access is easy, passing a reference to an actual game instance: Root, or (as I did) GameManager.
I chose for the latter solution, creating the new classes Bankruptcy and Mergers.
There is more stuff waiting to be looked at, for instance I found no less that 4 slightly different versions of the discardTrain() method, which in my opinion form another candidate for merging into one version.
Comments about this idea are welcome.
Given that this PR should make 1837 completely playable, or at least nearly so, I would suggest to release a new Rails version, so that people can start (or continue) to shoot at it. As new bugs have appeared until the very end, it is very likely that there are still some, so I have marked this as a beta version of 1837.
i'll push one shortly, Thank you Erik
During 1837 development, at several occasions I met a requirement to provide identical or very similar methods in different class hierarchies. Examples: (1) methods that would be identical for 1835 and 1837, but would have to be added to game-specific classes, like StockRound_1835 and StockRound_1837 because that code would be only relevant to these two games; (2) code related to share selling, that should derive both from Stock_Round_1837 and ShareSellingRound (unfortunately, java does not support multiple inheritance); (3) code that should be used both in SR- and in OR-derived classes.
In the past, I handled such cases by adding methods to the Round superclass (which, by the way, is no longer abstract). But other people did not like that, as is shown by several comments in that class. And I can understand that.
Another approach for which I found a few examples, is to create a separate class, having static methods only, so that access is easy, passing a reference to an actual game instance: Root, or (as I did) GameManager.
I chose for the latter solution, creating the new classes Bankruptcy and Mergers.
There is more stuff waiting to be looked at, for instance I found no less that 4 slightly different versions of the discardTrain() method, which in my opinion form another candidate for merging into one version.
Comments about this idea are welcome.
I do like this approach.
Game-dependent bankruptcy handling methods collected in a new class Bankruptcy. Reason: to avoid duplicate code.
Currently, it only includes the different ways to handle the train-buying company when a player goes bankrupt but the game continues. Three styles for now: 1835 (also used by 1837), 18EU and 18Scan.
Another new class Mergers is currently only used by 1837. Both classes only have static methods.
During testing, bugs were found in 18EU and 1835, which have been fixed.
A major problem arose in 1837: it crashed when a player going bankrupt still owned minors or coal companies. This has been fixed by exchanging these minors for major certificates. An exception to this rule applies to the Ug two-share minors U1 and U3. The exchange is only done if the bankrupt has both shares. If he has the president share only, the shares are swapped and the second minor share goes to the Pool, but will not be buyable. If the bankrupt has only the second minor share, that share will likewise go to the Pool.
Four test cases added, for different bankruptcy scenarios.
I will add some comments below about design decisions taken.