Step 6 of 6

Review: class design

Design bugs in class hierarchies are invisible in small tests and expensive in production. Three are common enough that experienced reviewers check for them on sight:

• a base class used polymorphically without a virtual destructor: deleting through a base pointer skips the derived destructor (leaking whatever it owns)
• storing derived objects by value in a container of the base type slices them: only the base part is copied, and virtual calls go to the base version
• an intended override with a slightly different signature (missing const) silently creates a new function instead. override turns that into a compile error

A correct hierarchy, for comparison

#include <iostream>
#include <memory>
#include <string>
#include <vector>

class Sensor {
public:
    virtual ~Sensor() = default;                       // 1. virtual destructor
    virtual std::string read() const { return "?"; }
};

class Thermo : public Sensor {
public:
    std::string read() const override { return "21C"; }   // 2. override checks the signature
};

class Humidity : public Sensor {
public:
    std::string read() const override { return "40%"; }
};

int main() {
    std::vector<std::unique_ptr<Sensor>> sensors;      // 3. pointers, so nothing is sliced
    sensors.push_back(std::make_unique<Thermo>());
    sensors.push_back(std::make_unique<Humidity>());
    for (const auto& s : sensors) std::cout << s->read() << " ";
    std::cout << "\n";

    Thermo t;
    Sensor copy = t;                                   // slicing: only the Sensor part is copied
    std::cout << copy.read() << "\n";
}

21C 40% 
?

The last two lines show slicing on purpose: copying a Thermo into a plain Sensor keeps only the base part, so read() gives the base answer. A std::vector<Sensor> does exactly that to every element pushed into it.

Reviewing a hierarchy

1. Does the base have a virtual destructor? If any function is virtual, it needs one.
2. Does every derived function that's meant to override say override? Add it, and let the compiler tell you about signature mismatches such as a missing const.
3. Are derived objects ever stored or passed by value as the base type? Switch to references, pointers, or std::unique_ptr<Base>.

Changing a function's parameter type (like the container in names) is a legitimate review fix when the old signature forces a bug.

Your turn: this pull request adds weapon logging. Find and fix all three problems so the tests pass.

Previous: Review: integer math