An answer to this question on Stack Overflow.

Question

This is related to the "Project Euler Problem 14"

As I think, my logic and the code is good enough,but it may take few seconds to give the answer. But when i run this code, the program stops when "starting_number" is around 103152(i can't remember the number exactly).

Can anybody please have a look at this code and, tell me where and what's wrong with this code.

Here's the code :

#include<stdio.h>
int starting_number;
int number_of_terms;
int j=0,k=0;
int term;
int main(){
  for(starting_number=2;starting_number<1000000;starting_number++){
    term = starting_number;
    number_of_terms = 1;
    while(1){
      {
        if(term%2==0){
          term = term/2;
          number_of_terms++;
        } else if(term%2!=0){
          term = 3*term + 1;
          number_of_terms++;
        }
      }
      if(term == 1) break;    
    }
                                 
    if(j>=number_of_terms)   //finding which chain is longer
      j=j;
    else if(j< number_of_terms) {
      j= number_of_terms;
      k=starting_number;
    }
    printf("\n%d",starting_number);
  } 
  printf("\n%d(%d)\n",k,j);
  return 0;
}

Answer

This one's mildly tricky, but your problem is here:

if(term == 1) break;

If the variable term becomes very large (as it can easily do) then it can overflow the int datatype.

When this happens term becomes negative. The C language modulus of a negative odd number is itself negative. Therefore, the end condition for your loop is never met.

Solve this problem by using a larger data type such as unsigned long long.

A less convoluted version of your code would appear as follows. Note that I have eliminated the global variables (those outside of your main function) because global variables are evil. I've replaced your infinite while-loop with a loop that uses an end condition. I've reduced duplication of code within the while-loop. I've eliminated the j=j case. Since printf is a slow function to run, I've commented out the prinft you had in the for loop, which improves the run-time significantly.

#include <stdio.h>
int main(){
  int number_of_terms;
  unsigned long long term;
  int j=0,k=0;
  for(int starting_number=2;starting_number<1000000;++starting_number){
    term = starting_number;
    number_of_terms = 1;
    while(term!=1){
      if(term%2==0)
        term /= 2;
      else
        term = 3*term + 1;
      number_of_terms++;
    }
    if(j<number_of_terms){   //finding which chain is longer
      j = number_of_terms;
      k = starting_number;
    }
    //printf("\n%d",starting_number);
  } 
  printf("\n%d(%d)\n",k,j);
  return 0;
}

And, indeed, using unsigned long long solves the problem.