4
\$\begingroup\$

My code is a general solution to the following problem from Project Euler #12:

What is the value of the first triangle number to have over five hundred divisors?

MY CODE

#include <iostream>
int generate_triangular(int n);
int enumerate_divisors(int n);
int main()
{
    std::cout<<"You want to find the first triangular number with less than or more than how many divisors? "<<std::endl;
    int user_input;
    std::cin>>user_input;
    bool result_found = false;
    int test_number = 1;
    int target;
    while(!result_found){
        int factor_count = enumerate_divisors(generate_triangular(test_number));
        if(factor_count < user_input){
            test_number++;
        }else{
            result_found = true;
            target = generate_triangular(test_number);
        }
    }
    std::cout<<"The "<<test_number<<" th triangular number"<<std::endl;
    std::cout<<target<<std::endl;
}
int generate_triangular(int n){
    return (n*(n+1))/2;
}
int enumerate_divisors(int n){
    int divisor_count = 0;
    for(int i = 1;i*i <= n;i++){
        if(n%i == 0){
            divisor_count++;
        }
    }
    return 2*divisor_count;
}

I found my solution to be quite fast for the purposes of the question (it found the first triangular number with over 500 divisors in about .2 seconds). Wondering how I can make it faster, and about any general code suggestions to be made.

\$\endgroup\$
2
\$\begingroup\$

Your naming of variables is hard to reason about. When I think about what your program is doing you, I come up with the following sequence of steps:

  1. Take in a target divisor count.
  2. Index through each triangular number n and check if it satisfies the condition.
  3. If it doesn't increment n and repeat.
  4. Once it does print out the results.

Here is how I would restructure your main loop:

std::cout << "You want to find the first triangular number with less than or more than how many divisors? " << std::endl;
int target;
std::cin >> target;
// Indexes the triangle numbers.
int n = 0;
// Keeps track of how many divisors.
int divisors = 0;
do {
    n++;
    divisors = enumerate_divisors(generate_triangular(n));
} while (divisors < target);
std::cout << "The " << n <<" th triangular number" << std::endl;
std::cout << generate_triangular(n) << std::endl;

Also, add some spaces in between your function declarations, it's hard to read when everything is clumped together. But don't over do it (in particular, don't double space everything).

\$\endgroup\$
2
\$\begingroup\$

Your enumerate_divisors(n) gives a wrong result if n is a perfect square.

Note that the question asks for a triangular number that has over 500 divisors, so you would have to enter 501 to perform the desired calculation.

I agree with @Dair that your main loop is convoluted. In particular, result_found is a flag variable, and you should structure your loops to eliminate flag variables. I would write it this way:

int divisor_threshold = …;
int i;
long t;
for (i = 1; divisor_count(t = triangular(i)) <= divisor_threshold; i++);
std::cout << "The " << i << "th triangular number " << t
          << " has over " << divisor_threshold << " divisors.\n";
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.