Fix a large class using inheritance

Question

It seems to me that my class is too big and complicated, I would like to reduce it. Can I use inheritance in this way, given that I created the InitCar class only to inherit it and am not going to use objects of this class explicitly.

Before refactoring. People and License are not my own classes, I cannot change them.

class Car
{
public:
    void Move();
    void SpeedUp();
    void SpeedDw();
    //More other

private:
    int speed = 0;
    std::string name;
    int id = 0;
    People owner;    // not my own class
    License license; // not my own class

    void InitCarFromConfig()
    {
        //Here I read the data from the file
    }

    void InitOwner()
    {
        //Here I init the People owner
    }

    void InitInspection()3
    {
        //Here I init the License license
    }


};

After refactoring

class InitCar
{
protected:
    std::string name;
    int id = 0;
    People owner;    // not my own class
    License license; // not my own class

    void InitCarFromConfig()
    {
        //Here I read the data from the file
    }

    void InitOwner()
    {
        //Here I init the People owner
    }

    void InitInspection()
    {
        //Here I init the License license
    }
};

class Car : InitCar
{
public:
    void Move()
    {
        InitOwner();
    }
    void SpeedUp();
    void SpeedDw();
    //More other
private:
    int speed = 0;
};

Is this use of inheritance acceptable and are performance issues possible?

Answer 1

Although it is true that you can use inheritance to reduce the size of you classes and reduce code duplication for other classes (which can also inherit InitCar for similar base functionality), it is actually not a good idea.

Functionality-wise, it does work, it does reduce code duplication and class sizes. However, it is a bad design, because it uses inheritance wrong, and breaks concepts of "clean code".

When you create a class, you create an entity which represents something. Inheritance create relations between those entities. Saying that Car inherits InitCar is saying that Car is a sort of InitCar, which logically makes no sense, because InitCar is just a helper class.

You can solve this if the base type was an actual entity, like Vehicle, and you had multiple vehicles. However, your InitClass is specifically intended to split you code, so it doesn't actually make a Vehicle, renaming it won't fix the design.

Composition over inheritance

A well known concept in "clean code", saying that it is better to hold functionality helping classes as variables in the class, rather then inherit from a base class. It is both more flexible for switching implementations, and does not abuse the purpose of inheritance:

class Car {
public:
    void Move();
    void SpeedUp();
    void SpeedDw();
    //More other

private:
    int speed = 0;
    std::string name;
    int id = 0;
    People owner; 
    License license;
    HelperClass helper; // new class for initing..

    void InitCarFromConfig()
    {
        //data = helper.InitCarFromConfig();
    }

    void InitOwner()
    {
        //owner = helper.InitOwnerForCar(param);
    }

    void InitInspection()
    {
        //data = helper.InitInspection(param);
    }
};

Now we simply delegate our calls to the helper class (whose name is just a stub, you should have a name matching what it does, or maybe several classes). So we do save some space, we don't abuse inheritance and typing, and we now actually have flexibility in implementation, since now we can replace the instance of helper and get a new logic.

How do we init helper? Usually receiving via the constructor is the best idea. But you can create inside the class if you really want to.

Best design for your case

But is it the best design here?! No actually. Because the real problem in the class is the existence of the Init_ methods within the class.

When creating a class, it is important that when the constructor finishes running, the class is completely initialized. When it is not, we create several problems:

• Risk a chance that the class is used, and some properties/methods are not complete for use, causing errors
• Complexity of the class is increased, making maintenance harder (which is what you pointed as the issue for you).
• Limits the users of the class from flexibility in its creation, thus limiting the design options for using the class. Creating tests for example, will be a nightmare.
• Very risk for multithreading use, and harder to handle

Instead than, what we can do is receive all the data we need to operate from the constructor, and simply store it:

public:
    Car(std::string name, id, People owner, License license);

Now comes another issue: what if it is difficult to perform initialization. After all you have 3 methods for initializing your class, so could be not easy for users. This is were the Factory design pattern comes in. We will create a class, named CarFactory (or so) and use it to create our classes. Within it, it will have all the logic to init the class data:

class CarFactory {
public:
    Car* CreateCar(params_from_user) {
        // init data
        return new Car(data);
    }
};

What did we accomplish with this:

• We made Car smaller and less complex
• We allowed users more options about how to use Car
• We maintained the Init option from earlier, to help create Car
• Our code is a lot easier to look at and maintain because it is separated logically, and classes are small
• Car is fully initialized after constructor call