Extra "0" in output when factorizing a number

Viewed 133

Write a function int fact(int n) which displays the factors of the integer n, and returns the number of factors. Call this function in main() with user input

#include<iostream>
using namespace std;

int fact(int n);

int main() {
    int n,factor;
    cout << "Enter an integer : ";
    cin >> n;
    factor = fact(n);
    cout << factor;
    return 0;
}

int fact(int n)
{
    for (int i = 1; i <= n; ++i) 
    {
        if (n % i == 0)
        cout << i << endl;
    }
    return 0;
}

If I enter 7, I get 1,7,0 . How do i remove this 0 and how do i find the number of factors?

5 Answers

The key part is "and returns the number of factors". You don't do that. Keep a count of the factors:

int fact(int n)
{
    int count = 0;
    for (int i = 1; i <= n; ++i) 
    {
        if (n % i == 0)
        {
            // found a factor, add to the count
            count++;
            cout << i << endl;
        }
    }
    // return the count instead
    return count;
}

Then, your main function can use that count:

factor = fact(n); // fact(n) will already print the factors
// now just print the number
cout << "Number of factors: " << factor << '\n';

You should count in your int fact() function. Set a variable to 0 and increment each time you currently display i. Then at the end of the function instead of returning 0 return the count variable.

int fact(int n)
{
    int  count=0;

    for (int i = 1; i <= n; ++i) 
    {
        if (n % i == 0) {
           cout << i << endl;
           count++;
        }
    }
    return count;
}
#include <iostream>
#include <vector>

std::vector<int> fact(int n);

int main() {
  int n;
  std::cout << "Number: ";
  std::cin >> n;
  std::vector<int> factors = fact(n);

  for (auto i : factors) {
    std::cout << i << ' ';
  }
  std::cout << '\n';

  std::cout << "Number of factors: " << factors.size() << '\n';

  return 0;
}

std::vector<int> fact(int n) {
  std::vector<int> vec{1};
  for (int i = 2; i <= n / 2; ++i) {
    if (n % i == 0) {
      vec.push_back(i);
    }
  }
  vec.push_back(n);

  return vec;
}

If you're going to return anything from fact(), it should be the factors. To do so, I am using a std::vector. It is an array that can grow on demand. The numbers 1 and n are always factors, so I don't bother doing the math for them. The vector is initialized already holding the value 1, and I only calculate numbers up to and including half of n (Anything greater than n/2 won't divide evenly, so my loop is finished about half as fast by recognizing the actual range). I then just add n to the vector, which I return.

My main prints the vector, and the vector knows its own size, which is the number of factors.

Alternatively, you can just keep a count in your fact() function.

#include <iostream>
#include <vector>

// Prints factors of n and returns the number of factors
int fact(int n);

int main() {
  int n;
  std::cout << "Number: ";
  std::cin >> n;
  int numFactors = fact(n);

  std::cout << "Number of factors: " << numFactors << '\n';

  return 0;
}

int fact(int n) {
  int factorCount = 2;  // Already counting 1 and n
  std::cout << "1 ";
  for (int i = 2; i <= n / 2; ++i) {
    if (n % i == 0) {
      std::cout << i << ' ';
      ++factorCount;
    }
  }
  std::cout << n << '\n';

  return factorCount;
}

The main problem with your code is that your function always returns zero. You need to keep a count of factors and return it.

Besides that your code performance badly as the loop goes on much longer than needed. You can use the square root of n as the limit in the for loop. Like:

int fact(int n)
{
    if (n < 1) return 0;

    int res = 0;
    int limit = sqrt(n);
    for (int i = 1; i <= limit; ++i) 
    {
        if (n % i == 0)
        {
            res += 2;
            cout << i << " - " << n/i << endl;
        }
    }
    if (limit * limit == n)
    {
        --res;
    }
    
    return res;
}

For n = 36 the output is:

1 - 36
2 - 18
3 - 12
4 - 9
6 - 6

and the returned value is 9

Below is another approach. It doesn't use square root. Instead it keeps the number of loops low by using the square of i as loop limit.

int fact(int n)
{
    if (n < 1) return 0;

    int res = 0;
    int i = 1;
    int i_square = i * i;
    while (i_square < n)
    {
        if (n % i == 0)
        {
            res += 2;
            cout << i << " - " << n/i << endl;
        }
        
        ++i;
        i_square = i * i;
    }
    if (i_square == n)
    {
        ++res;
        cout << i << " - " << n/i << endl;
    }
    
    return res;
}

Fact() always returns 0 so this line print 0

  cout << factor;

for the number of factors you can change the return value of fact() :

int fact(int n)
{
    int nb = 0;
    for (int i = 1; i <= n; ++i) 
    {
        if (n % i == 0) {
           cout << i << endl;
           nb++;
        }
    }
    return nb;
}
Related