search

mannyzhou5 / evolutionchamber

Automatically exported from code.google.com/p/evolutionchamber
0 stars 0 forks source link

Boilerplate code in EcAction subclasses #86

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
All EcAction subclasses have a very similar code. Most of the differences are 
in used resources. The code can be greatly simplified by moving execute and 
isPossible methods to EcAction class. 
EcAction will have protected members (or getter methods) for each available 
resource (mineral, drone, etc..) with zero value. Execute and isPossible 
methods will be part of EcAction class. Subclasses will overwrite these members 
(or methods) with resource values as required to inject specific values.
The Runnable class can be created in EcAction by using reflection to get 
variable name (e.g. for s.roaches += 1 ) or an instance variable. It is advised 
however to move Runnable class creation to a separate method since otherwise it 
will be problematic to override it in subclasses if needed. 

This change will make EcAction subclasses much smaller and easier to maintain 
since most of the logic will be written once in EcAction class.

P.S. I noticed that EcActionUpgrade class actually does something similar, but 
on a smaller scale.

Original issue reported on code.google.com by sergey...@gmail.com on 26 Oct 2010 at 7:28

GoogleCodeExporter commented 9 years ago

Original comment by azzur...@gmail.com on 26 Oct 2010 at 8:27

GoogleCodeExporter commented 9 years ago 
If you're going to refactor actions, it will improve speed to take out all the 
anonymous classes and make them static nested classes.

Original comment by brendan....@gmail.com on 26 Oct 2010 at 8:46

GoogleCodeExporter commented 9 years ago 
It is unclear if static nested classes provide any benefit. See here 
http://stackoverflow.com/questions/758570/is-it-possible-to-make-anonymous-inner
-classes-in-java-static

Original comment by sergey...@gmail.com on 26 Oct 2010 at 10:44

GoogleCodeExporter commented 9 years ago 
sergeykad,
Could you provide a patch that demonstrates this?

Original comment by mike.angstadt on 17 Nov 2010 at 3:46

GoogleCodeExporter commented 9 years ago 
I'll try to create a patch till Sunday to demonstrate the change I have in 
mind. It will include 2-3 classes as proof of concept only (and therefore will 
break compilation of the rest of EcAction subclasses).

Original comment by sergey...@gmail.com on 17 Nov 2010 at 4:42

GoogleCodeExporter commented 9 years ago 
That's ok.  I think that it would help a lot to see a solid example.  Thanks a 
lot.

Original comment by mike.angstadt on 17 Nov 2010 at 10:58

GoogleCodeExporter commented 9 years ago 
I checked the code and I see Fritley already implemented changes very similar 
to one I have in mind. I guess the issue can be closed.

Original comment by sergey...@gmail.com on 19 Nov 2010 at 8:34

GoogleCodeExporter commented 9 years ago 
Setting as fixed by Fritley upon sergeykad's request.

Original comment by netpr...@gmail.com on 19 Nov 2010 at 9:39