Chief Delphi - Is my code good?

Chief Delphi (http://www.chiefdelphi.com/forums/index.php)

- Java (http://www.chiefdelphi.com/forums/forumdisplay.php?f=184)

- - Is my code good? (http://www.chiefdelphi.com/forums/showthread.php?t=149171)

Is my code good?

Here's a link for my team's 2016 java program: https://github.com/olidem123/PLS5618

I was just wondering if there was ways to improve/make it lighter (other than clarity :cool: ).

PS: We are a french canadian team so most of the program is in english. Ask me if you have questions on words.

thx

Re: Is my code good?

There is lots to like about your code. It's well organized and generally easy to read. A couple specific comments:

1. We like to use constants in RobotMap.java, organized by kind of interface. We can then print this out and use it as instructions for wiring.

For example:

Code:

         /** 

          * PWM            */ 

        public static final int CHASSIS_JAGUAR_FRONT_LEFT = 2; 

         public static final int CHASSIS_JAGUAR_BACK_LEFT = 3; 

         public static final int CHASSIS_JAGUAR_FRONT_RIGHT = 0; 

         public static final int CHASSIS_JAGUAR_BACK_RIGHT = 1; 

         public static final int BALL_SHOOTER_LEFT_SPEED_CONTROLLER = 4; 

         public static final int BALL_SHOOTER_RIGHT_SPEED_CONTROLLER = 5; 

         public static final int ARM_EXTEND_SPEED_CONTROLLER = 6; 

         public static final int CAMERA_TILT_MOTOR = 7;

I'd then recommend moving the declarations out of RobotMap and into the subsystem that owns the device/controller. We generally make each controller/encoder/gyro be owned by exactly one subsystem, and hide the reference inside the subsystem (make them private). This way we know that there's only one place to look for code that changes those entities.

You're almost there with your final references in the subsystems, but they are copies of reference and the actual controller/encoder/gyro is available to everyone.

2. In AvancerHerse, consider using more descriptive variable names:

Code:

        int h1 = 950;

        int h2 = 450;

and think about simplifying "!(Robot.bras.distPot() < h1)" a bit:

Code:

                if (!Robot.pelle.herseAcotee()) {

                        // h1, h2 = hauteur pelle

                        if ((Robot.bras.distPot() > h1)) {

                                Robot.chassis.reculer(-0.4, 0);

                        } else if ((Robot.bras.distPot() > h2) && !(Robot.bras.distPot() < h1)) {

                                Robot.chassis.reculer(-0.6, 0);

                        } else {

                                Robot.chassis.reculer(-0.9, 0);

                        }

                }

3. In BrasCommand, you say:

Code:

Timer.delay(0.005);

Wondering why this is necessary here and in the other places in the code. Delay like this is needed only in very specialized cases.

4. In BrasCommand, you didn't put this in braces:

Code:

                } else

                        Robot.bras.controlBras(0);

For student code, I recommend that every block be enclose in braces as it's consistent and simple to think about.

5. Generally you want the same code in end() and interrupted() -- see Reculer.

Re: Is my code good?

Quote:

Originally Posted by MrRoboSteve (Post 1594537)

For student code, I recommend that every block be enclose in braces as it's consistent and simple to think about.

Not just student code - many company coding standards require this as well, including Java code written for/at Google.

My team intends to use Google's style guide for all our code in the future. Consistently applying rules and formatting will help with integrating and understanding code written by multiple people, and it helps students get used to the idea that they will eventually have to work in an environment where such rules are imposed.

olidem123, it might be a good resource for you too :)

Re: Is my code good?

Just some git/project related things:

I recommend you name your project something that includes the year and/or game. If you're going to be making another robot program next year, having separate names will allow you to store both projects on GitHub. For example, naming it PLS5618-Stronghold-2016 will allow you to make another project next year: PLS5618-WaterGame-2017 :)

Make sure you allow your .gitignore to take effect. On your local copy of the project, commit anything if necessary, then run

Code:

git rm -r --cached .

git add .

git commit -a -m "Untrack gitignored files"

---

Instead of having the command pair InBallon/OutBallon, which are very similar, I think it makes more sense to have a single command handle both, eg

Code:

public class MoveBall extends Command {

        public enum MoveBallDirection {

                IN,

                OUT

        }



        MoveBallDirection whichDirection;

        public MoveBall (MoveBallDirection whichDirection) {

                requires(Robot.pelle);

                this.whichDirection = whichDirection;

        }



        protected void initialize() {



        }



        protected void execute() {

                if (whichDirection == MoveBallDirection.OUT) {

                        Robot.pelle.outBallon();

                } else {

                        Robot.pelle.inBallon();

                }

                Timer.delay(0.08);

        }



        protected boolean isFinished() {

                if (whichDirection == MoveBallDirection.OUT) {

                        return !Robot.oi.stick.getRawButton(4);

                } else {

                        return Robot.pelle.limitFond();

                }

        }

}

Similarly, you could even refactor Pelle.in/outBallon into a single function.

Re: Is my code good?

I just edited the program. https://github.com/olidem123/PLS5618-2016

Quote:

1. We like to use constants in RobotMap.java, organized by kind of interface. We can then print this out and use it as instructions for wiring.

I changed those things a bit and it looks a lot better, still need to test it. Thx

Quote:

2. In AvancerHerse, consider using more descriptive variable names:

I just removed this part of the program because we found out it was useless.

Quote:

4. In BrasCommand, you didn't put this in braces:

Code:

                } else

                        Robot.bras.controlBras(0);

Guess I forgot them ;)

Quote:

Instead of having the command pair InBallon/OutBallon, which are very similar, I think it makes more sense to have a single command handle both, eg

I use 2 commands because I found it easier to set up 2 buttons to one command each.

Quote:

Not just student code - many company coding standards require this as well, including Java code written for/at Google.

I'll take a good look at that ;)

PS: I sent this post to our mentors for tips for the future years. Thx for all your support!

Re: Is my code good?

Quote:

Originally Posted by olidem123 (Post 1595291)

I use 2 commands because I found it easier to set up 2 buttons to one command each.

Well you'd have a constructor parameter that specifies which direction to roll the ball.

Code:

public enum Direction {

    IN,

    OUT

}

// ...

public class BallCommand extends Command {

    Direction whichDirection;

    public MyCommand(Direction whichDirection){

        this.whichDirection = whichDirection;

    }

    // ...

}

// ...

// then in OI

Button buttonA = new JoystickButton(stick, 1),

       buttonY = new JoystickButton(stick, 4);

buttonA.whenPressed(new BallCommand(Direction.IN));

buttonY.whenPressed(new BallCommand(Direction.OUT));