CODEWITHSUNDEEP
c++for-loopbreak
Joey C.
Joey C.

Reputation: 87

Finding Prime Numbers using For Loops in C++

This is my first post here, so I hope I am doing everything correctly.

I have a bit of a problem with my most recent programming challenge. The goal is to have the user input a number and have the program output all of the prime numbers between 0 and the number entered. My strategy was to have a for loop run a simple check on every number from 1 to the number entered by dividing it by every preceding number until the loop reached 1. If at any point in the check, the program encountered a number that divided into the number that the loop is currently on, that section of the loop was to "break" and continue on to the next number. If the divisor reached 1 then the program would have successfully determined that nothing could divide into the original number evenly and output that number as being prime.

I am very new to programming, but I think it's a problem with how I'm using break. When the program is compiled, it only prints the original number that was entered and nothing else. However, I have checked to make sure that all of my integers and loops are working. Any help regarding my aforementioned problem or if I can make my code more efficient or "correct" would be appreciated. 

#include "stdafx.h"
#include <iostream>

int _tmain()
{
    using namespace std;
    int iUserInput;
    cin >> iUserInput;

    for(;iUserInput > 0; iUserInput--)
    {
        int iDivisor = iUserInput - 1;
        for (; iDivisor > 0; iDivisor--)
        {
            if (iUserInput%iDivisor == 0)
                break;
            if (iDivisor == 1)
                cout << iUserInput << endl;
        }
    }


    return 0;
}

Upvotes: 2

Views: 1447

Answers (3)

sankalpn
sankalpn

Reputation: 76

Look at what order your if conditions are getting executed. When iDivisor reaches 1 you break instead of printing. Either 1) swap your if conditions or 2) modify inner loop to terminate when iDivisor reaches 1 and move second if condition outside inner for loop.

Upvotes: 0

Christian Hackl
Christian Hackl

Reputation: 27528

The following part:

if (iDivisor == 1)
    cout << iUserInput << endl;

is at the wrong position. You need to put it outside of the inner loop:

for(;iUserInput > 0; iUserInput--)
{
    int iDivisor = iUserInput - 1;
    for (; iDivisor > 0; iDivisor--)
    {
        if (iUserInput%iDivisor == 0)
            break;
    }
    if (iDivisor == 1)
        cout << iUserInput << endl;
}

Other observations:

  • #include "stdafx.h" is completely unnecessary here. Remove it.
  • int _tmain() is not standard. Make it int main() and make sure it compiles like this.
  • iUserInput is Hungarian Notation. Consider it a failed Microsoft experiment from the 90s which is completely useless in modern C++. Call the variable userInput.
  • Changing the value of the userInput variable may hurt readability of the code. After all, the user input does not change after it has been entered. Consider counting down a copy of userInput, e.g. for(int count = userInput; count > 0; count--).
  • Make your other for loop more compact, too: for (int divisor = userInput - 1; divisor > 0; divisor--).
  • Perhaps too advanced for someone who really just started learning C++, but you should try to handle illegal user input, e.g. someone entering "X" instead of a number. You can check the state of std::cin after each input operation to see if the last operation succeeded or not. Otherwise your program may exhibit undefined behaviour.

Upvotes: 2

Jan Buchar
Jan Buchar

Reputation: 1322

You're breaking too soon :) when iDivisor is 1, iUserInput%iDivisor must equal zero, and the second condition never gets evaluated. Just swap the two if statements and you should be good.

Upvotes: 0

Related Questions