Search code examples
c++classaggregationinitialization-list

C++ initialization list, Class in class (Aggregation)


I am writing a dish washer program, Dishwasher has a pump, motor, and an ID. Pump, motor, date, time are other small classes which Dishwasher will use. I checked with the debugger but when I create the Dishwasher class, my desired values aren't initialized. I think I am doing something wrong but what? :(

So the Dishwasher class is below :

class Dishwasher {
    Pump pump; // the pump inside the dishwasher
    Motor motor;// the motor inside the dishwasher

    char* washer_id;//011220001032 means first of December 2000 at 10:32h
    Time time_built;// Time variable, when the Dishwasher was built
    Date date_built;// Date variable, when the Dishwasher was built

    Time washing_time;      // a time object, like 1:15 h
public:
    Dishwasher(Pump, Motor, char*, float);
}; 

This is how I initialize the Class :

Dishwasher::Dishwasher(Pump p, Motor m, char *str, float f) 
    : pump(p), motor(m), max_load(f)
{
    washer_id = new char [(strlen(str)+1)];
    strcpy (washer_id,str);
    time_built.id2time(washer_id);
    date_built.id2date(washer_id);
}

This is how I create the class:

Dishwasher siemens( 
        Pump(160, "011219991143"), 
        Motor(1300, "081220031201"), 
        "010720081032", 
        17.5);

Here is the full code if you would like to see further, because I removed some unused things for better readability : http://codepad.org/K4Bocuht


Solution

  • First of all, I can't get over the fact you're using a char* for shuffling strings around. For crying out loud, std::string. It's a standardized way of dealing with a string of characters. It can wiggle its way out of anything you throw at it, efficiently. Be it a string literal, a char array, a char* or yet another string.

    Second of all, your professor needs professional help. It's one thing to teach the "lower level intuition" with something more raw, but to enforce this on students who ought to be studying C++ is plain bad practice.

    Now, onto the problem at hand. I'll just take the toplevel example, namely the raw, "naked" pointer lying in your Dishwasher constructor, the char* str. As you know, that's a pointer, namely a pointer to a type char. A pointer stores the memory address of a variable (the first byte of any type of variable in question, which is the lowest addressable unit in memory).

    That distinct difference is very important. Why? Because when you assign a pointer to something else, you're not copying the actual object, but only the address of its first byte. So, in effect, you just get two pointers pointing to the same object.

    As you are, no doubt, a good memory citizen, you probably defined a destructor to take care of this:

    washer_id = new char [(strlen(str)+1)];
    

    You're basically allocating strlen(str)+1 bytes on the heap, something that is unmanaged by the system and remains in your capable hands. Hence the name, a heap. A bunch of stuff, lose the reference to it, you'll never find it again, you can just throw it all away (what actually happens to all the leaks when the program returns out of main). Therefore, it is your duty to tell the system when you're done with it. And you did, defined a destructor, that is.

    A big, nasty but...

    But... There is a problem with this scheme. You have a constructor. And a destructor. Which manage resource allocation and deallocation. But what about copying?

    Dishwasher siemens( 
            Pump(160, "011219991143"), 
            Motor(1300, "081220031201"), 
            "010720081032", 
            17.5);
    

    You are probably aware that the compiler will try to implicitly create a basic copy constructor, a copy assignment operator (for objects that have already been constructed) and a destructor. Since an implicitly-generated destructor has nothing to release manually (we discussed dynamic memory and our responsibility), it is empty.

    Because we do work with dynamic memory and allocate varying size blocks of bytes to store our text, we have a destructor in place (as your longer code shows). That is all well, but we still deal with an implicitly generated copy constructor and a copy assignment operator which copy the direct values of variables. As a pointer is a variable whose value is a memory address, what an implicitly-generated copy constructor or copy assignment operator does is just copying that memory address (this is called a shallow copy) which just creates another reference to a unique, singular byte in memory (and the rest of the contiguous block).

    What we want is the opposite, a deep copy, to allocate new memory and copy over the actual object (or any compound multi-byte type, array data structure etc.) stored at the incoming pointer's memory address. This way, they will point to distinct objects whose lifespan is tied to the lifespan of the enveloping object, not the lifespan of the object from which is being copied from.

    Observe in the example above that you're creating temporary objects on the stack which are alive for the time the constuctor is in action, after which they are released and their destructors are invoked.

    Dishwasher::Dishwasher(Pump p, Motor m, char *str, float f) 
        : pump(p), motor(m), max_load(f)
    {
        washer_id = new char [(strlen(str)+1)];
        strcpy (washer_id,str);
        time_built.id2time(washer_id);
        date_built.id2date(washer_id);
    }
    

    The initialization list has an added benefit of not initializing objects to their default values and then performing the copy, since you have the honor of directly calling the copy constructor (which in this case is implicitly generated by the compiler):

    pump(p) is basically invoking Pump::Pump(const Pump &) and passing in the values the temporary object has been initialized with. Your Pump class contains a char*, which holds the address of the first byte to the string literal you shoved into the temporary object: Pump(160, "011219991143").

    The copy constructor takes the temporary object and copies all the data explicitly-available to it, that means it just takes the address contained in the char* pointer, not the entire string. Therefore, you end up pointing from two places to the same object.

    Since the temporary objects live on the stack, once the constructor finishes dealing with them, they will be released release and their destructors invoked. These destructors actually destroy the string along with them, which you placed while creating the Dishwasher object. Now your Pump object within the Dishwasher object holds a dangling pointer, a pointer to a memory address of an object lost in the endless memory abyss.

    Solution?

    Write your own copy constructor and copy assignment operator overload. On the example of Pump:

    Pump(const Pump &pumpSrc) // copy constructor
    
    Pump& operator=(const Pump &pumpSrc) // copy assignment operator overload
    

    Do this foreach class.

    Of course, in addition to the destructor, which you already have. These three are the protagonists over a rule of thumb called "The Rule of Three". It states that if you have to declare any of them explicitly, you probably need to explicitly declare the rest of them, too.

    The probably part of the rule is just a void of responsibility, basically. The need for explicit definitions is actually quite obvious when you get further down the road with C++. For example, thinking about what your class does is a good way of determining whether you need everything defined explicitly.

    Example: Your classes depend upon a naked pointer which points to a piece of memory and the memory address is all it pulls along. It's a simple typed variable which holds only a memory address to the first byte of the object in question.

    In order to prevent a leak upon destroying your object, you defined a destructor which frees the allocated memory. But, do you copy the data between objects? Very likely, as you've seen. Sometimes you're going to create a temporary object which will allocate data and store the memory address into a pointer data member whose value you will copy. After a while, that temporary is bound to be destroyed at some time, and you will lose the data with it. The only thing you'll have left is the dangling pointer with a useless and dangerous memory address.

    Official disclaimer: Some simplifications are in place to prevent me from writing a book on the subject. Also, I always try to concentrate on the question at hand, not the code of the OP, which means I do not comment on the practices employed. The code might be horrible, beautiful or anything in-between. But I do not try to turn the code or the OP around, I just try to answer the question and sometimes suggest something I think could be beneficial to the OP in the long run. Yes, we could be precise as hell and qualify everything... We could also use Peano axioms to define the number sets and the basic arithmetic operations before we boldly try to state that 2 + 3 = 3 + 2 = 5, for the same amount of fun.