2

Why does the for loop in the while loop works just once? Also, please tell me if my logic of finding prime numbers is correct. The code only displays 2 as output.

#include <iostream>
using namespace std;

int main()
{
    int x, z = 0;
    int y = 0;
    int a = 2;

    cout << "Input the number of prime numbers you need: ";
    cin >> x;
    while (x > 0)
    {
        for (int i = 1; i < 11; i++) 
        {
            z = a % i;
            if (z == 0)
            {
              y = y + 1; 
            }
        }

        if (y < 3)
        {
            cout << a << ", " << endl;
            x = x - 1;
        }
        a = a + 1;
    }

    return 0;
}
asynts
  • 2,213
  • 2
  • 21
  • 35
Basit
  • 33
  • 4

2 Answers2

1

Your basic mistake is that you do not reset your variable y to zero after each iteration of the while. It can only grow, and when the number 3 is tested, it already is big enough not to let any number pass, thus it passes only the number 2.

Quick fix: after

    if (y < 3)
    {
        cout << a << ", " << endl;
        x = x - 1;
    }
    a = a + 1; 

insert

    y = 0;

Another bug is that you only check for numbers smaller than 11, which means that for example 121 (11*11) will be a false positive, your program will think it is prime when it isn't.

That said, you should learn how to write code that is easier to debug by yourself, especially using a debugger. If you never have used a debugger, don't worry, you'll easily find tutorials for that. In your case, the debugger would have showed you the state of y and you'd have seen that it reaches numbers it shouldn't be able to.

Two general hints for future code:

  1. Use descriptive variable names. x and y sound to me like coordinates, but they are not. Give them proper names, like number_dividers instead of y, for example.

  2. Break your code into parts. For example, have a function like this:

    #include <cmath>
    
    bool is_prime(int number)
    {
        if(i <= 1) return false;
    
        for(int i = 2; i <= std::sqrt(number); i++)
        {
            if(number % i == 0) return false;
        }
        return true;
    }
    

Having such functions makes your code more tidy, allows you to reuse parts of the code, and, especially in your case, allows you to test this unit of code in isolation, that is testing if for a given input it has the right output (essentially a unit test).

Your code would now be:

#include <iostream>
using std::cout;
using std::cin;
using std::endl;

//(insert code from above here)

int main()
{
    int required_primes = 0;
    cout << "State the amount of prime numbers to be found: ";
    cin >> required_primes;

    int found_primes = 0;
    int current_number = 2;
    while(found_primes < required_primes)
    {
        if(is_prime(current_number))
        {
            cout << "Found prime: " << current_number << endl;
            found_primes++;
        }
        current_number++;
    }
}

(The descriptive variable names make it far easier to understand for somebody looking at the code for a first time, wouldn't you agree?)

Aziuth
  • 3,652
  • 3
  • 18
  • 36
  • Thank you so much Aziuth! You explained it so well. Your suggestions certainly will improve the code and make it easier for someone to interpret. – Basit Nov 21 '18 at 12:22
-1

This will solve the issue.

 #include <iostream>
 using namespace std;

int main()
{
    int x, z = 0;
    int a = 2;

    cout << "Input the number of prime numbers you need: ";
    cin >> x; 
    while (x > 0) 
    {
        bool isPrime = true;
        for (int i = 2; i < a; i++) 
        {
            z = a % i;
            if (z == 0)
            {
                isPrime = false;
            }
        }

        if (isPrime)
        {
            cout << a << ", " << endl;
            x = x - 1;
        }

        a = a + 1; 
    } 
    return 0;
}
Chameera
  • 82
  • 8
  • 2
    Could you please elaborate on *why* this code solves the issue? – Bob__ Nov 20 '18 at 10:13
  • To find an prime value, we need to check whether the value X is divisible by any number between 2 and X-1, So `for (int i = 1; i < 11; i++)` is an invalid statement and it should be `for (int i = 2; i < a; i++) ` or more effectively `for (int i = 2; i <= sqrt(a); i++) ` – Chameera Nov 21 '18 at 11:32