I finished my review too.. But, then I got inspired to hack some of the changes I thought hackyInstaller needed. I did some quick hacking on my _local_version_ and put it on the web < http://csdl.ics.hawaii.edu/~kagawaa/hackyInstaller-1.0.809_src.zip >.
Note: that I just did this for fun... But, also to provide some suggestions. So take a look if you want. I didn't change everything. I just did some of the commands and the CLI and left out the swing view component (which probably a lot more complex compared to the CLI interface). But, it compiles, tests, and runs ok.
Here are some of the major changes:
1) I made Control.java a Singleton and improved the control flow a little
/**
* Main method used to start the hackyInstaller application.
* @param args Command line arguments for the CLI.
* args[0] is reserved for the command.
* args[1...n] is reserved for the arguments.
*/
public static void main (String[] args) {
Control control = Control.getInstance();
if (args.length == 0) {
control.launchSwingInstaller();
}
else {
control.isCliRunning = true ;
List arguments = new ArrayList();
//adds args to an array list
for (int i = 1; i < args.length; i++) {
arguments.add(args[i]);
}
control.launchCliInstaller(args[0], arguments);
}
}
2) Control executes the command automatically instead of relying on the view components to execute the command.
/**
* Executes a command.
* @param request The request.
*/
public void executeCommand(Request request) {
try {
String commandClassName = "org.hackystat.kernel.installer.controller.command."
+ request.getCommand() + "Command";
Command command = (Command) Class.forName(commandClassName).newInstance();
command.execute(request);
}
catch (Exception e) {
// Log exception
HackyInstallerLogger.getInstance().logException(e.getMessage(), e);
if (this .isCliRunning) {
this .notifyModelListeners( "The " + request.getCommand()
+ " command is invalid. Please note that the commands are case sensitive.");
new Cli( "-Help", new ArrayList());
}
}
}
3) Remove 2 of the methods in the Command class. Execute(request) is the only class left. Also, the instance variables in the commands seemed a little scary, so the one method removes the need for instance variables. a single entry point into the commands allows the commands to validate the request arguments in their own unique way.
/**
* Executes the checking of the the Hackystat host and key
* @param request the request.
*/
public void execute(Request request) {
if (this .validateArguments(request)) {
String host = (String) request.getArguments().get(0);
String key = (String) request.getArguments().get(1);
try {
SensorPropertiesReader reader = new SensorPropertiesReader(host, key);
reader.validateHackyInfo(); //checks if host/key are valid and writes out to file
}
catch (ModelException e) {
//Log exception
HackyInstallerLogger.getInstance().logException(e.getMessage(), e);
Control.getInstance().notifyModelListeners( new ModelEvent(e.getMessage(), false ));
}
}
}
4) the Cli changed; the requested command is automatically executed via the Control. Which got rid of the strange if(command != null) check. . Hopefully, the Swing GUI could be changed just as easily. If the request is invalid then an error message is printed only in the CLI view.
* @param command the command
* @param arguments the command line arguments.
*/
public Cli(String command, List arguments) {
//adding listeners
Control.getInstance().addModelListener( this );
Control.getInstance().addDownloadListener( this );
Request request = new Request(command, arguments);
Control.getInstance().executeCommand(request);
}
Anyway... those were some thoughts on how the HackyInstaller code could be improved.
thanks, aaron
At 01:15 PM 8/9/2005, you wrote:
BTW, I've just finished my review and imported the review/ directory to cvs, so do an update to get it.
Cheers,
Philip
--On Tuesday, August 09, 2005 1:12 PM -1000 Julie Ann Sakuda <[EMAIL PROTECTED]> wrote:
Hello all,
Just a reminder that the HackyInstaller code review is tomorrow at 12:30. The module
is called hackyInstaller2 on hackydev.
~Team InstaHack!
