southwestroboticsprogramming / 2022_water_game Goto Github PK

I just looked over the RobotBase class and there are some C++ usage recommendations:

1. RobotBase needs to have virtual destructor. The basic rule is if a class any virtual functions, it should have a virtual destructor. Otherwise, you'll just get static typing and if you delete from a RobotBase pointer, the destructor of the derived class won't be called, and at best you'll only have a memory leak.
2. Related: the destructor is usually grouped in with copy constructor and assignment operator because if you need to define one, you need to define all of them. Usually, that's because of resource management, but for this class is more about usage. Unlike java, C++ was always paradigm, so when you define a class it might be primarily part of polymorphism - like RobotBase or any java class - or it might be a value type. For value types, C++ is trying to make it possible to make your object act just like a primitive. You can declare them as locals that live on stack and are cleaned up on scope exit, you use them as a return value, you can copy and assign.

And that gets into the special copy constructor. X(const X&) and assignment operator: X& operator=(const X&). C++ will create one for you if you don't provide one. But these are are statically scoped, if you assign to a RobotBase it will just copy the base part which you've defined as no-ops.

And you don't want that, in fact it's even better to prevent it happening by accident. So you should add RobotBase& operator=(const RobotBase&) = delete; and RobotBase(const RobotBase&) = delete; to your class. (C++ is actually a little biased toward value types, that why functions aren't virtual by default, among other things).

3. If you did want to be able to create copies of the Robot you'd define a virtual RobotBase* clone() = 0; member, and probably make the copy constructor protected to help with the implementation. Then you could define KillerRobot* KillerRobot::clone() { return new KillerRobot(*this); }
4. I did notice a factory method. RobotBase* createRobot() -- something like that. In modern C++, we don't use raw pointers much. That function would almost always be set up to return std::unique_ptr<RobotBase> instead. If you haven't learned about _smart _pointers : std::unique_ptr and std::shared_ptr you should.

That's enough for now.