1. FileAction.java:
FileAction an insanely huge class at 1600+ lines of code. MainMenu has the same
problem, and we've talked about this before. You guys are going to have a hell
of a time writing documentation describing what these classes do. A Javadoc of
FileAction is going to be ugly. Try consolidating your code into smaller
specialized classes, don't repeat yourself in the method, class, package, or
code base (DRY principle), and don't rely so much on static methods in the code
base - you lose the leverage of object-oriented programming when you do.
Example:
public static FileAction.saveScenarioFile() has nothing to do with the class
description that's put in line 58. It's an io helper.
Why not make a FileMaker class then invoke it when you need it? In fact, I
think Apache FileUtils.write() is a static method that does this already and we
have that jar in the lib. Leverage as much existing code, don't keep writing
the same thing over and over.
Finally, isn't there a cleaner way than if-else statements to map actions to
business logic? This architecture makes it hard to find things and will be hard
to maintain. Struts2 is a web app framework that does this (see struts.xml).
There may be something similar for Swing. The logic for each action, I would
think, would go in it's own class.
4. ScenarioMonitor.java
Line 54 is redundant -- Tad comment: This boolean is already initialized as
false. WON'T FIX - line 54 can be reached in more than one way so saving may
need to be rest.
ScenarioMonitor.lastLine()
Why not use Apache's FileUtils.readLines()
then grab the last element?
Or at least break it out into a separate class so it can be used elsewhere in
the code base.
Original issue reported on code.google.com by Tad.Slawecki@gmail.com on 23 Aug 2013 at 4:37
Original issue reported on code.google.com by
Tad.Slawecki@gmail.com
on 23 Aug 2013 at 4:37