View Single Post
  #3   Spotlight this post!  
Unread 12-02-2012, 03:27
ItzWarty ItzWarty is offline
Registered User
FRC #1072 (Harker Robotics)
Team Role: Leadership
 
Join Date: Jan 2012
Rookie Year: 2010
Location: California
Posts: 15
ItzWarty is an unknown quantity at this point
Re: How is this looking?

Consistency is key in making maintainable code, which will save you from headaches in the future.

Not all of the following things can be done immediately, since programming is something that takes time to learn.

1) Having a standardized naming convention will make code much more readable, and thusly maintainable by larger teams. For example, the code contains the following:
1a) VariableName, variableName
1b) METHODNAME (which to a some would look like some sort of macro), methodName
1c) CONSTANT_NAME, ConstantName
2) Breaking up your code into organized classes increases code maintainability (You have object oriented programming! Take advantage of that!)
3) Use the features available in your language.
One thing that really caught my attention is this:
Code:
public void timerControl(String args) {
        if(args.equals("stopAndReset")) {
            timer.stop();
            timer.reset();     
        }else if(args.equals("Start")) {
            timer.start();
        }else if(args.equals("stopAndReset")) {
            timer.stop();
            timer.reset();
        }
    }
This isn't maintainable. Someone somewhere is going to accidentally capitalize something and then wonder "Gee, why isn't timerControl doing anything?" Consider using something similar to an enumeration (look at how WPILibJ does it with things like Relay.Value.*)
4) Consider designing the "contract" which comes with a method. When I call something named "Get Emergency", I expect it tells me something along the lines of "Is there a critical failure?" or "Is the emergency button pressed?". I don't expect it to actually mutate the state of the robot (GetEmergency can stop the drive train)
5) I should note that
Code:
        while(getBrake() == false && getEmergency() == false)
is equivalent to the more readable
Code:
        while(!getBrake() && !getEmergency())
Best wishes,
ItzWarty

Last edited by ItzWarty : 12-02-2012 at 03:35.
Reply With Quote