Object oriented design issue in C++

Question

Suppose I have a car, having an engine and a radio. I want to start the radio when engine starts, and want to destroy car when engine is over a certain temperature. In the book Thinking in C++, there was a composition example that helped me out. My idea was similar:

class Engine
{
public:
    int temperature;
    void start()
    {
        temperature = 15
    }

    void rev()
    {
        temperature += 2;
    }
}

class Radio
{
public:
    void open() {}
}

class Car
{
public:
    ~Car();
    Engine engine;
    Radio radio;
}

I know I can manually implement:

main()
{
    Car car;
    car.engine.start();
    car.radio.open();
    while(car != NULL)
    {
        car.engine.rev();
        if(car.engine.temperature > 50)
        {
            delete(&car);
        }
    }
}

I may have syntax errors, it's not important. My question is: How can I create all the communication between classes automatically, so that

main()
{
    Car car;
    while(car!= NULL)
    {
        car.engine.rev();
    }

does the same job? I also tried inheritance and virtual functions to create the communication, but failed.

Answer 1

Observer pattern

You could use an Observer pattern, which provides a general way for the Engine to pass a notification to an arbitrary interested party (often you'd allow multiple observers who'd all be notified, but for this purpose supporting one is enough).

struct IEngineObserver
{
    virtual void on_overheated() = 0;
};

class Engine
{
  public:
    Engine(IEngineObserver& observer) : observer_(observer) { }

    void start()
    {
        temperature_ = 15;
    }
    void rev()
    {
        if ((temperature_ += 2) > 50)
            on_overheated();
    }
    IEngineObserver& observer_;
    int temperature_;
};

class Radio
{
  public:
    void open() {}
};

class Car : public IEngineObserver
{
  public:
    Car() : engine_(this), finito_(false) { }
    ~Car();
    virtual void on_overheat() override { finito_ = true; }
    Engine engine_;
    Radio radio_;
    bool finito_;
};

You can then loop until car.finito_ is true. It doesn't make sense to delete the Car from within the Engine though, if you have some code in main() continually invoking operations on the Car object... that will crash. The Car will be destroyed when it goes out of scope at the end of main, so your focus should be on breaking out of the rev()ing loop at the right time - i.e. when finito_ is set.

Lambda and std::function

Something similar can be done with lambdas and std::function, so that the Car can specify arbitrary code for the engine to run when it overheats as in...

    Car() : engine_([&] () { finito_ = true; }), finito_(false) { }

    Engine(std::function<void (*)()>& f) : f_(f) { }

...without needing an extra IEngineObserver class or on_overheat functions.

Answer 2

You seem to be confusing a couple of issues. First of all, you say you want to delete a class from within itself. That is normally not a good idea.

The key here is that there is a difference between allocating and deallocating memory, and having a valid object. You could have a bool overheated variable in your class that you will set to true when the Car overheats. Then your loop can look like this:

class Engine {
private:
    bool overheated;
public:
    bool isOverheated() const { return overheated; }
    void rev();

    // Rest of implementation ...
};

void Engine::rev() {
    temperature += 2;
    if (temperature > 50) overheated = true; 
}

class Car { /* ... */ };

int main() {
    Car car;
    while (!car.engine.isOverheated()) {
        car.engine.rev();
    }
}

Using NULL, delete and so on is related to memory allocation. For a class like this, you should normally separate concerns. Memory allocation (stack vs. heap) is one thing, and whether the object is in a correct state or not is another.

Edit:

If you want the Car's state to be dependent on the Engine's, then just do like this:

class Car {
    Engine engine;
public:
    bool isWorking() const { return !engine.isOverheated(); }
};

Answer 3

First of all

There are many wrong things in your approach,

Most important (I think) : you seem to confuse the notion of pointer to a dynamically allocated object and local declared object. In your code Car car; is not a pointer. Therefore it cannot be NULL and you can't delete it.

I show you with an exemple

int main()
{
  // this is a pointer unitialized, so it points a random place in memory
  // (depending on what was in memory before) 
  Car * car;
  
  // initialization at NULL
  // the pointer points to NULL, i.e no object
  car = NULL;   

  // a car object is created in memory and the pointer `car` now points on it
  car = new car();

  // do things with your object through the pointer
  car->engine.start();
  
  // the car object is deleted from memory and Car points to NULL
  delete(car);
   
  return 0;
}

Now without pointers (which is a better way to do things in C++)

int main()
{
   // car is a local object, it is automatically created by calling the default
   // constructor
   Car car;

   // do things with your object
   car.engine.start();

   return 0;
   
} // <- the oject is automatically destroyed when you go out of the scope

My example is surely not exhaustive about the difference between pointer and local object. I strongly recommand you to read more about Oriented Object Programming and Its notions (such as encapsulation, because in your case it's not good too).

And keep in mind that C++ does not the things the way others OO languages can do, like java for example where every Object variable works like a pointer.

However

Concerning your original problem about behavior of objects and relationship between them, the answer of Lstor is a good exemple of how a OO design can look like. But I think you have misunderstooding on OO programming that lead you to that kind of bad design, thus my advice is to read (or carefully re read, or choose another book) more on OO programming in C++

Answer 4

It is possible, even though not recommended in the general case, to delete an object from within itself. You can do it like this:

class A
{
public:
    void commitSuicide()
    { 
        delete this; 
    }
}

However, in your current design, there are several issues that prevent you to do this:

1. You are using an object in the stack, i.e. not a pointer to an object allocated in the heap with new. Hence, it is forbidden to call delete.

2. The engine is not aware of the car it belongs to. Hence, in engine::rev(), calling delete this; would delete the engine, not the car. An option is to add a member car * theCarIBelongTo to engine, so that you can call delete theCarIBelongTo;. An other option is to have a method car::revEngine() that calls Engine.rev();, then calls delete this; if the temperature is too high. (so you call Car->revEngine() instead of Car->Engine.rev() )

3. Last but not least, deleting the object would not result in the pointer pointing to the object becoming NULL, and hence your test Car != NULL would still succeed. This will lead to Undefined Behaviour when deferencing it (in practice, a segfault). This is the most serious issue, mainly the reason why "comitting suicide" is often not recommended. You would need to change your design to take it into account.

Hence, even though my answer is a most direct answer to your question, I would not recommend you to use the "commiting suicide" paradigm in this case. Study carefully the other answers provided, they are the most useful and provides you good practices.

Answer 5

"Suppose I have a car, having an engine and a radio. I want to start the radio when engine starts, and want to destroy car when engine is over a certain temperature"

start the radio when engine starts:from engine:

ok, add a radio field to engine and call the open method from engine::start

destroy car when engine is over a certain temperature

ok, when you increase the engine temperature in engine rev, after the line,

temperature += 2;                          /// add this line
if (  temperature > 50  ) car.destroy();

engine will need to have a car to destroy so make sure your engine looks like this,

class Engine
{
public:
    int temperature;
    car car;
    void start()
    {
        temperature = 15
    }

    void rev()
    {
        temperature += 2;
        if (  temperature > 50  ) car.destroy();
    }
}



main()
{
    Car car; 
    CAR.ENGINE.CAR = CAR;
    CAR.ENGINE.RADIO = CAR.RADIO;

    while(car!= NULL)
    {
        car.engine.rev();
    }

C'a froid, NON?