What is wrong with this constructor definition?-CodePudding

I want to write a class of "Clock", which basically holds hours, minutes and seconds (as integers).

I want that by default the constructor would initalize hours,minutes and seconds to 0 unless other input was given. So I declared the following:

    Clock(int hour=0, int minute=0, int second=0);

Which basically should give me 4 different constructors - the first one is the default constructor, and then a constructor which recieves hours as an input, a constructor which recievrs hours and minutes as an input, and a constructor which recieves all 3 as an input.

Next, I want the constructor to initalize the fields only if the input is valid, meaning, hours should be between 0 to 24, minutes and seconds should be between 0 to 60. So I wrote 3 functions that checks if the input is legal, and then I implemented the constructor as the following:

Clock::Clock(int hour , int minute, int second ){
    if (isHourLegal(hour) && isMinLegal(minute) && isSecLegal(second)){
        time[0] = hour; time[1] = minute; time[2] =second;
    }
    else
        Clock();
}

Where time[] is an array of length tree which holds the hours,minutes and seconds fields of the object.

Basially, if one of the inputs is wrong, Im calling Clock(); which should create an object with the default intefers I wrote when I declared.

Is there something wrong with the way I implemented the constructor? Im having unexpected outputs when I try to print an object fields.

Also,

Assume I fixed my constructor to:

Clock::Clock(int hour , int minute, int second ){
    if (isHourLegal(hour) && isMinLegal(minute) && isSecLegal(second)){
        time[0] = hour; time[1] = minute; time[2] =second;
    }
    else
        time[0] = 0; time[1] = 0; time[2] =0;
    }
}

Im trying to call the default constructor in my main function by :

Clock clock1();
clock1.printTime();

(printTime is a method which prints the fields).

But I get an error. Is there a porblem using the constructor this way?

CodePudding user response：

You can solve this easily.

If you have default arguments, chances are these will pass the test (you define them, after all :-)). So why don't take advantage of this and just write a single constructor that handles both cases?

In both cases (i.e. with/without passing arguments to it), the checks will be performed and on success a valid Clock will be constructed. For your default arguments, this will be trivially true. In the failure case, your constructor will throw and you won't be having a half-initialized/broken object roaming around.

Maybe the point which you are missing is: If the time format is not legal, you should usually throw.

Look at the following definition, which should be sufficient for your case.

// Declaration
Clock(int hour=0, int minute=0, int second=0);

// ...

// Definition
Clock::Clock(int hour, int minute, int second)
  : time { hour, minute, second } // assuming int time[3]; here
{
    if (!isHourLegal(hour) || !isMinLegal(minute) || !isSecLegal(second)){
        throw std::invalid_argument("Illegal time format!");
    }
}