0

I'm using C. I expected this condition in the while loop to prevent any even value bigger than 4 million from being summed in my "sum" variable:

while ( (box1 < 4000000) || (box2 < 4000000) || (box3 < 4000000)) {

    box3 = box1 + box2;
    if (box3 % 2 == 0) /* checks if box is even */
        sum += box3;  /* if TRUE adds even value to the variable sum */

    box1 = box3 + box2;/* exceeded 4 millions on 12th loop */
    if (box1 % 2 == 0)
        sum += box1;

    box2 = box3 + box1; 
    if (box2 % 2 == 0)
        sum += box2;/* then summed undesired value here */
    printf("%d\n", sum);
}

It didn't come to my mind at all the possibility of the boxes exceeding 4 millions in the first or second statement within the loop. That happened and it took me a good while till I figure my program was summing one extra value before the while checks the condition again. I hope you to forgive me and have a little patience for making you spend your precious time here but I need to know if experienced programmers know how to avoid such simple, but annoying errors.

If that is true I would be very glad for having an opportunity to learn from better programmers. Thanks

Ezequiel Barbosa
  • 211
  • 1
  • 15
  • 1
    First of all, consider what happens if e.g. `box2` is over 4 million. Because you're using OR, "box1 is okay OR box2 is okay OR box3 is okay" evaluates to "okay". You will break your `while` only when *all three* are too big. – Amadan Apr 20 '15 at 03:37
  • Is the only constraint that the sum variable should not be added with a even value greater than 4 million?? Is there some constraint on max value of sum? – Msk Apr 20 '15 at 03:37
  • 7
    If you can find a way to avoid making mistakes when writing code then you are going to have a very long and successful career. :) – Sam Apr 20 '15 at 03:38
  • Secondly, please tell us what the program is supposed to do, as we can't meaningfully fix otherwise. – Amadan Apr 20 '15 at 03:38
  • google "boolean logic" and "truth tables". – 1.618 Apr 20 '15 at 03:50
  • 2
    Stop trying to fix the code. That's not the question. He is asking about programming practices. – kaylum Apr 20 '15 at 03:56
  • @AlanAu: Ah, right. Okay then, it's off-topic, and should be moved to Programmer's Stack Exchange. But in a nutshell, you can't discover logic errors automatically (or you'd need telepathic computers). The best you can do is to be attentive, step through your code in your head, create test cases that look for edge conditions, imagine all the worst scenarios, and structuring algorithms to minimise points of failure. – Amadan Apr 20 '15 at 04:02
  • @Amadan Thanks for answering me. Please notice that I already fixed the program, that, as you asked me, has a goal of summing all even Fibonacci's terms below 4000000. I solved it by using an IF statement like this `if (box1 % 2 == 0 && box3 < 4000000)` – Ezequiel Barbosa Apr 20 '15 at 04:02
  • In this specific code, "minimising points of failure" would be a good step to take. Instead calculating three Fibonacci per cycle, calculate one and then move the variables around. This way, you only need to test at one point, instead of at three different ones. – Amadan Apr 20 '15 at 04:04
  • I have been programming for over 40 years. I still learn new things on a (nearly) daily basis. Experience will be your biggest/best teacher. One thing that I have found to really help is to 1) really know the system functions I'm using 2) write out (pseudo code) the functions I'm writing, before putting anything onto the code/implementation. 3) expect there to be bugs, even in the simplest code – user3629249 Apr 20 '15 at 04:05
  • Part of the answer is 'experience'. I'm not sure what you really expected to happen (and we can't see the starting values for `box1`, `box2`, `box3` or `sum`). One thing is that the loop condition is only evaluated as control flows past it (some beginners expect the loop condition to be evaluated at all points in the loop. Another is that the condition only fails when all three variables are too big; if you wanted it to stop when any one was too big, you have to use `&&` instead of `||`. Getting that sort of logic right is something that comes with experience too. (And others that don't fit.) – Jonathan Leffler Apr 20 '15 at 04:05
  • @Msk Yes, that's the only constraint. Notice the code's already fixed and running. What I'm asking really is about good practices in algorithm development, so you can find possible errors before actually coding wrong code – Ezequiel Barbosa Apr 20 '15 at 04:06
  • @JonathanLeffler, I didn't catch this OR thing at all. Ok, I think my question is somewhat answered. `experience == good_coding` – Ezequiel Barbosa Apr 20 '15 at 04:17
  • Like I said, I'm not sure why you expected `box1` to stay under 4 million. At some point, it was going to exceed it, because the loop won't stop until all the box variables are over 4 million, and the box variables won't exceed 4 million until an addition in the body of the loop makes the value larger than 4 million. And, as I said, with more experience, you'll come to know how to write what you want/mean — and your expectations will come to match what you write. But since you've got some sort of answer that helps, I'll stop trying to extend my semi-answers. – Jonathan Leffler Apr 20 '15 at 04:25

1 Answers1

1

To showcase a correct way to think about this, as noted in comments - the technique here is to recognise that you are repeating the same code snippet three times and that two out of those three are not checked properly. It is usually called the DRY principle ("Don't Repeat Yourself"). Having noticed this, you refactor your code so that "it is DRY". A DRY code has less points of failure than non-DRY one, which obviously reduces the possibility of an error.

int previous = 1, current = 1;
int next;
int sum = 0;

// single test for end
while (current < 4000000) {

    // test for evenness
    if (current % 2 == 0) {
        sum += current;
    }

    // next value
    next = previous + current;

    // shuffle values around so you can reuse the same code
    previous = current;
    current = next;
}

Also note that if the variables are named box1, box2 and box3, no-one can read your code comfortably - and that includes your future self. Depending on the complexity, in one to six months you will have no idea what you wrote; having sensible names is pretty crucial for smooth maintenance.

However, note that these are only some of the principles of good code. Most of them come, as noted by many commenters, through experience. You start noticing "code smell", and when you do, you refactor it so it doesn't smell any more.

(Also, as noted in comments, general questions on the programming craft should go to https://softwareengineering.stackexchange.com/ ; Stack Overflow is more focused on specific problems. And if you do a search of that site, there are a number of threads that discuss good or maintainable code.)

Community
  • 1
  • 1
Amadan
  • 191,408
  • 23
  • 240
  • 301