What is wrong with this constructor definition?

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?

>Solution :

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!");
    }
}