Time function sometimes works sometimes doesn't

Question

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

Wow, this is pretty hard to read :o Is it a WTF coding-style ? — Boiethios, Apr 20 '16 at 07:56
I'm talking about your code formatting. Pick one formatting style (for exemple https://www.kernel.org/doc/Documentation/CodingStyle ) ; your code will be much more readable. Here, it seems your indentation is random. — Boiethios, Apr 20 '16 at 08:03
The logic is easy to understand, he is talking about everything else :) (indentation, coding style, ... Maybe it's because you used copy-paste but I hope your real code is "more beautiful" than that. It'll be easier for you to understand, and it's easier to find bugs or error with well written code. :) — Raph Schim, Apr 20 '16 at 08:04
You need to reformat your code before we can help you. As it stands -- you have an unmatched close brace on line 7. We can't determine if this is part of your problem or just an artifact of your cut-and-paste until you clean things up. — user590028, Apr 20 '16 at 08:22
@RaphaelSchimchowitsch So i don't make the mistake again, are you talking about the indenting that i have done? I ask so i don't make the mistake again — Bad programmer, Apr 20 '16 at 17:53

fluter · Answer 1 · 2016-04-20T08:40:36.647

3

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.

edited Apr 20 '16 at 08:40

answered Apr 20 '16 at 08:30

fluter

13,238
8
62
100

thank you for the suggestion of turning the time into seconds, i have applied that logic and it works perfectly. Once again your help is much appreciated. – Bad programmer Apr 21 '16 at 08:43
@Badprogrammer glad to know it helped. – fluter Apr 21 '16 at 08:48
can you point to where i might have gone wrong pls min = (time4 * 10) + time3; sec = (time2 * 10) + time1; total = (min * 60) + sec; total = total - 1; //Total stores the number of seconds if (total == 0) finish = 1; //converting seconds back into min and seconds min = total / 60; time4 = min / 10; time3 = min % 10; temp1= total - min * 60; time2 = temp1 / 10; time3 = temp1 % 10; – Bad programmer Apr 22 '16 at 07:37
@Badprogrammer what are you trying to do? – fluter Apr 22 '16 at 07:41
i'm trying to convert the input into seconds and then subtract it by 1, then convert it back to a readable time. It doesn't work for some reason – Bad programmer Apr 22 '16 at 08:09
@Badprogrammer check out http://ideone.com/2Gbcw7 you were using the wrong variables. – fluter Apr 22 '16 at 08:20

molbdnilo · Answer 2 · 2016-04-21T09:16:14.680

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 time_k is when time_k-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)
            {
                // ...
            }
        }
    }
}

Ahahah, that makes sense. I would have tried to debug it myself but unfortunately the debugger on MikroC is not as good as the debugger on Visual studios. Once again thank you for your help — Bad programmer, Apr 21 '16 at 08:44
@Badprogrammer You don't need a debugger for problems like this. You only need basic understanding of program flow and a few minutes of thinking. — molbdnilo, Apr 21 '16 at 09:16

Time function sometimes works sometimes doesn't

2 Answers2