View Single Post
  #9   Spotlight this post!  
Unread 05-22-2016, 07:45 PM
euhlmann's Avatar
euhlmann euhlmann is offline
CTO, Programmer
AKA: Erik Uhlmann
FRC #2877 (LigerBots)
Team Role: Leadership
 
Join Date: Dec 2015
Rookie Year: 2015
Location: United States
Posts: 298
euhlmann has much to be proud ofeuhlmann has much to be proud ofeuhlmann has much to be proud ofeuhlmann has much to be proud ofeuhlmann has much to be proud ofeuhlmann has much to be proud ofeuhlmann has much to be proud ofeuhlmann has much to be proud of
Re: Code Review: Semi-Novice

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
Code:
if(pSwitch.get() == true) { ... }
You'd do
Code:
if(pSwitch.get()) { ... }
And instead of
Code:
if(RobotMap.intakeOutSwitch.get() == false) { ... }
You'd do
Code:
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)

Last edited by euhlmann : 05-22-2016 at 07:48 PM.
Reply With Quote