Any suggestions to improve some simple code?

I’ve been attempting to add PID to our robot this off season. It works fine, we just need to tune a bit more. Here is the code. I would genuinely appreciate any suggestions or tips to make the code “better” in any way. For example, would one of you write something differently than I did? if so, why? https://github.com/frcteam4189/OffSeason_Project/tree/master/src/main/java/frc/robot

Thanks in advance for any suggestions!

First, you do a really good job of having very little code in your main Robot.java class and do a great job of separating the code into many classes in the standard structure that is recommended.

In your initTalon method, I HIGHLY recommend that you call TalonSRX#configFactoryDefault(). So if one time you set some crazy config to a Talon, you won’t have to worry about that carrying over.
So that would be motor.configFactoryDefault() on the first line of that method.

3 Likes

The only thing that really worries me about the code is that all the fields inside the DriveTrain class are declared to be public. I strongly suggest making them private. Code outside the subsystem should never need to directly access the internal implementation of the subsystem. Instead, they should interact with the subsystem via the public methods of the subsystem. That way, you can change the subsystem implementations (hardware and/or algorithms) and the rest of the code base should not need to change.

For example, in Robot.robotInit(), you have this line:
driveTrain.ahrs.reset();

It would be better if it was something like:
driveTrain.resetGyro();

Where DriveTrain.resetGyro() contains:
ahrs.reset();

This way, you could change gyro types without affecting anything outside the subsystem.

Hope this helps,
Steve

3 Likes

Looks awesome overall! I wish half the teams code I looked at was as clean as this. Heck, I wish my own code was this clean. or simple. or straightforward.

I’m scraping the bottom of the barrel to find anything to make better.

To be super nit-picky:

In your constants file, your names have inconsistent order. kLeftEncoder and kDriveLeft for example. The general rules of thumb I use for cases like this:

  1. Order words from “generic” to “specific”.
  2. The evil you know (ie what’s there already) is better than creating a new evil (ie, introducing a new standard).

Some constants don’t mention what their units are. I usually do this in a comment or in the variable name itself.

Commented out lines can generally be deleted, unless you frequently flip back and forth between commentd and uncommented. Since you’re using git, you can always use its ability to go back in time to see what the code used to be.

Consider using more detailed and organized commit messages.

Again though, super awesome job.

This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.