Member variable/method access control

[vmagro]

As a general rule of thumb, everything other than static variables should always be declared as private. If you need to access or set the value of a class's member variable, you should have getter and setter methods to access/set the value respectively.

Here are some spots where I noticed you either didn't declare an access modifier or used it incorrectly:

• the drive mode booleans
  • should be private and have getters/setters if necessary
• the constructor of Drive
  • The constructor of Drive should be declared as private, not public
  • This might not make immediate sense but the reason for making it private is to enforce that no one else can create a Drive instance except code in the Drive class. Since a robot can only have one drivetrain, you should employ the Singleton pattern.
  • For an example of a Singleton, see this part of the Drive code from 2014
• gyroEnabled
  • should be private and have getter/setter if necessary
• driveHalo and driveTank
  • Since these methods are public, anyone can call them to control the drivetrain, completely bypassing the current mode selection. This is allowing other code to potentially circumvent your selected driving mode, which could cause hard to debug issues.
  • They should both be private and only called from the public drive() method that will then call the appropriate choice based on the current driving mode.
• setDriveshifter
  • this should obviously be private for the same reason as directly above
  • you even say not to call it in the comment right above the method declaration
  • you shouldn't just "ask nicely" to not call it, you should not allow anyone to call it at all by declaring it private
• straightPidEnabled
  • should be private and have getter/setter if necessary

[vmagro]

One more thing: the skim method in Drive should also be private, right now it's the default, which means it will be able to be called from any other class in that package.