Skip to content

Milestone 2.03 code review comments not yet addressed #173

@GoogleCodeExporter

Description

@GoogleCodeExporter

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions