Allowing a robot to drive around

Background:

I'm a member of my high school's robotics club (and in charge of the
programming team). The robot is going to be written in C++, but I'm mostly
a Python coder and want to make sure I don't have any fundamental gaps in my
knowledge.

In preparation for the start of the competition in just a few days, I've
quickly written up the bare minimum required for a robot to drive around
(included below). I was hoping to get feedback and criticism at this stage
to nip any problems/misconceptions I have before they get serious.

As previously stated, I'm unfamiliar with C++ so don't know many of its best
practices/idioms/etc. What I'm looking for is a bit vague, but in essence,
how can I make my code more professional/better? Am I making any errors?

Any feedback would be welcome.

There are three files:

• MainRobot.h

• MainRobot.cpp

• typedefs.h

MainRobot.h

#ifndef MAINROBOT_H_
#define MAINROBOT_H_

// 3rd-party modules
#include "WPILib.h"

// Modules created by us
#include "typedefs.h"

class MainRobot : public SimpleRobot
{
public:
    // Required methods (for robot to run)
    MainRobot(void);
    void Autonomous(void);
    void OperatorControl(void);

    // Objects (provided by WPILib.h)
    RobotDrive robotDrive;
    Joystick *leftStick;
    Joystick *rightStick;
    Timer timer;

    // Motor ports (hardware)
    static const UINT32 kLeftFrontMotor = kPWMPort_1;
    static const UINT32 kRightFrontMotor = kPWMPort_2;
    static const UINT32 kLeftBackMotor = kPWMPort_3;
    static const UINT32 kRightBackMotor = kPWMPort_4;

    // Joystick ports (hardware)
    static const UINT32 kRightJoystickPort = kUSBPort_1;
    static const UINT32 kLeftJoystickPort = kUSBPort_2;

    // Delay to prevent spamming motors with input
    static const float kDelayValue = 0.01;
};

#endif /*MAINROBOT_H_*/

MainRobot.cpp

```
#include "MainRobot.h"

/**
* The constructor.
*
* Initializes the motors, drivetrain, joysticks, safety devices, etc.
*/
Main

Do you plan to replace the Joystick object?

Joystick *leftStick;
Joystick *rightStick;

If not then declare them as

Joystick  leftStick;
Joystick  rightStick;

If you are going to replace them then you should be using smart pointers rather than a RAW pointer (if you want to do it manually then learn the rule of three) But I would go with smart pointers (but that is a second option to normal objects).

If you must have pointer use smart pointers:

std::unique_ptr  leftStick;
std::unique_ptr  rightStick;

There is no need declare these as static (unless you want to get the address or something) so just use enum:

// Motor ports (hardware)
static const UINT32 kLeftFrontMotor = kPWMPort_1;
static const UINT32 kRightFrontMotor = kPWMPort_2;
static const UINT32 kLeftBackMotor = kPWMPort_3;
static const UINT32 kRightBackMotor = kPWMPort_4;

Prefer:

enum { kLeftFrontMotor = kPWMPort_1, kRightFrontMotor = kPWMPort_2, kLeftBackMotor = kPWMPort_3, kRightBackMotor = kPWMPort_4};

No need to explicitly return from a constructor:

MainRobot::MainRobot(void)
{
    ...

    return;  // No need
}

There is nothing wrong with prefixing functions with the class name but personally I would not.

while(MainRobot::IsAutonomous()) {
    MainRobot::GetWatchdog().Feed();
    Wait(kDelayValue);
    // Do nothing.
}

Easier to write:

while(IsAutonomous())
{
    GetWatchdog().Feed();
    Wait(kDelayValue);
    // Do nothing.
}

The file name "typedefs.h" is relatively common. You may want to avoid it.

StackExchange Code Review Q#7334, answer score: 2