Code Review: Semi-Novice

[Ben Wolsieffer]

Originally Posted by Lireal

The major issue I see with your code is that you are using the sybsystems and RobotMap completely wrong. Your RobotMap should only really include numbers. All of your motor controllers, sensors, etc. should be a in your subsystems. Read through these examples and you will see how to do it: https://wpilib.screenstepslive.com/s.../13809/c/88893

RobotMap does not necessarily need to only have numbers. In fact, RobotBuilder puts all the motor controller definitions in RobotMap, and then includes references to them in each subsystem. I don't necessarily think this is the best way to do it, but I would definitely say that it is not "wrong" to put things other than numbers in RobotMap.

[euhlmann]

This isn't really a code thing, but it's best practice in git to exclude anything binary (your bin, build, and dist) folders. You can use a .gitignore file to do this.

General
- You've got some redundant/unnecessary classes. Instead of things like IntakeIn and IntakeOut you could have a generic IntakeRoll that accepts an enum parameter for in or out.
- When using booleans in if blocks, you don't need an "== true"
So instead of

if(pSwitch.get() == true) { ... }

You'd do

if(pSwitch.get()) { ... }

And instead of

if(RobotMap.intakeOutSwitch.get() == false) { ... }

You'd do

if(!RobotMap.intakeOutSwitch.get()) { ... }

OI.java
- Using an array of joystick buttons might be neater

Robot.java
- What is pSwitch? A better variable name would help. Also the channel is hardcoded
- You store joystick buttons here. Those should be in OI

RobotMap.java
- You name some channel numbers "driveVictorX" and then create a Talon object with them
- Then you name your lift motor channel "liftTalon" and create a Victor from it
- Intake switches are hardcoded

Subsystems
- I agree with the design choice of having subsystems store hardware objects (instead of RobotMap), but it's really up to you.

Note that these are best practices/recommendations. It's difficult to write software well, and even our own code doesn't really meet these standards (especially as we piled on more and more mid-competition hacks)