CODEWITHSUNDEEP
c++cmikroc
Bad programmer
Bad programmer

Reputation: 46

Time function sometimes works sometimes doesn't

So I am trying to make a time function that counts down. It's based on something I saw here. The variables are given from a different function. Once time runs out the variable finish is turned to 1 and it leaves the function. This function works sometimes and sometimes it doesn't work, for example if i give it an input of 11sec it works fine but if i give it 1:00 min it doesn't work. Can someone tell me what's wrong with the code. 

if (time1 == 0 && time2 == 0 && time3 == 0 && time4 == 0)
//if all the time is 0 finish the sequence
    finish = 1;

if (time1 != 0) //Checking to see if the first digit is NOT at 0
    time1 = time1 - 1; //  subtract time 1 by 1
else {
    time2 = time2 - 1;  //When time1 is 0
    time1 = 9;
} //Time1 going back to it's original value

if (time2 == 0 && time1 == 0) { //if time1 and time2 are 0s
    if (time3 != 0) { //The minute value (time3)
        time2 = 5;  //60 SECONDS
        time3 = time3 - 1;
        time1 = 9;
    }             
} //Put time 1 to its original value
if (time2 <= 0 && time1 <= 0 && time3 <= 0) {
    if (time4 != 0) { //The minute value (time3)
        time2 = 5; //60 SECONDS
        time3 = 9;
        time4 = time4 - 1;
        time1 = 9; 
    }
} //Put time 1 to its original value

Time4 = 3, Time3 = 2, Time2 = 1, Time1 = 0. This will mean that the time is at 32:10 min

Upvotes: 1

Views: 327

Answers (2)

molbdnilo
molbdnilo

Reputation: 66431

The problem is that you're comparing to zero after having changed a number to be non-zero.

Assuming that 1:00 is encoded as

time1 = 0
time2 = 0
time3 = 1

you can follow your own logic:

if (time1 != 0) // Nope
    time1 = time1 - 1;
else { // Yes
    time2 = time2 - 1;
    time1 = 9;
}

Now you have

time1 == 9
time2 == 0
time3 == 1

if (time2 == 0 && time1 == 0) { // Nope, time1 is 9
    if (time3 != 0) {
        time2 = 5;  
        time3 = time3 - 1;
        time1 = 9;
    }             
}

and you still have

time1 == 9
time2 == 0
time3 == 1

and finally

if (time2 <= 0 && time1 <= 0 && time3 <= 0) { // Nope
    if (time4 != 0) { 
        time2 = 5; 
        time3 = 9;
        time4 = time4 - 1;
        time1 = 9; 
    }
}

so you end up with

time1 == 9
time2 == 0
time3 == 1

that is, 1:09.

The only time you want to change timek is when timek-1 has "crossed" the zero.
This can be done with a nest of conditionals:

if (time1 > 0 || time2 > 0 || time3 > 0 || time4 > 0)
{    
    time1 -= 1;
    if (time1 < 0)
    {
        time1 = 9;
        time2 -= 1;
        if (time2 < 0)
        {
            time2 = 5;
            time3 -= 1;
            if (time3 < 0)
            {
                // ...
            }
        }
    }
}

Upvotes: 2

fluter
fluter

Reputation: 13806

You cannot just check against non-zero, you need to check if given time is positive, otherwise you are subject to count with negative values, and the counters might overflow.

if (time1 > 0)
    time1 -= 1;
if (time3 > 0)
    time3 -= 1;

Another thought, you are counting down with each digit of minutes and seconds, why not just use seconds, by converting your time into seconds. For example, to count down on 1:23:

int minutes = 1;
int seconds = 23;
int timer = minutes * 60 + seconds;

// in your timer function
if (seconds == 0) {
    finish = 1;
} else if (seconds > 0) {
    seconds -= 1;
} else {
      // error
}

This way it would be also extensible, what if you want to handle hours, just add hours * 3600 to seconds, you can do this easily to handle days, months even. In your approach, add those would result too many cases, they are nearly impossible to handle correctly.

Upvotes: 3

Related Questions