Prime-testing program not working

Question

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

using namespace std;

int main ()
{
    int x;
    cout << "Enter a number." << endl;
    cin >> x;
    int y = 1;
    int i = 0;
    while (i == 0 && y < sqrtf(x))
    {
        if (fmodf(x,y) == 0)
        {
            i = 1;
        }
        else
        {
            i = 0;
        }
        y++;
    if (i == 1)
    {
        cout << "Your number is prime." << endl;
    }
    else 
    {
        cout << "Your number is composite." << endl;
    }
    }
    return 0;
}

This is a code I created to test for primes. After sorting out several debugging issues I was able to run it.

It opened the command window, read 'Enter a number' and closed itself the second I entered a number.

Help?

Why are you using `fmodf` for that? – AnT stands with Russia Mar 12 '16 at 21:35 — AnT stands with Russia, Mar 12 '16 at 21:35

score 1 · Answer 1 · edited May 23 '17 at 11:59

You have to:

close the while loop in the correct place
change the if (i == 1) condition (i==1 means x is divisible by some y)
start with y = 2 (every number is divisible by one)
include sqrtf(x) in the loop (y <= sqrtf(x) or 15, 25, 35... are primes).

So:

int main()
{
  int x;
  cout << "Enter a number." << endl;
  cin >> x;

  int y = 2;  // <-- changed
  int i = 0;

  while (i == 0 && y <= sqrtf(x))  // <-- changed
  {
    if (fmodf(x,y) == 0)
    {
        i = 1;
    }
    else
    {
        i = 0;
    }

    y++;
  }  // <-- moved here

  if (i == 0)  // <-- changed
  {
    cout << "Your number is prime." << endl;
  }
  else
  {
    cout << "Your number is composite." << endl;
  }

  return 0;
}

works (more or less...).

Anyway:

don't use using namespace std; (Why is "using namespace std" considered bad practice?)
\n should be your default ("\n" or '\n' or std::endl to std::cout?)
y and x are integers so you can use % instead of fmodf
avoid premature pessimization: prefer preincrement, only use postincrement if you're going to use the original value
else { i = 0; } is superfluous
you can change y < sqrtf(x) with y * y <= x (and you don't need math.h anymore) or find square root of number then start the loop

Somewhat better (but far from perfect):

#include <cmath>
#include <iostream>

int main()
{
  int x;
  std::cout << "Enter a number.\n";
  std::cin >> x;

  int square_root = std::sqrt(x);
  int y = 2;
  int i = 0;
  while (i == 0 && y <= square_root)
  {
    if (x % y == 0)
      i = 1;

    ++y;
  }

  if (i == 0)
    std::cout << "Your number is prime.\n";
  else
    std::cout << "Your number is composite.\n";

  return 0;
}

Now:

input validation, i.e. check for bad input values (How to check if input is numeric in C++)
special cases checking (is the number one a prime number?)
a bool would express your intentions better than int i;
improve the algorithm (Determining if a number is prime, Which is the fastest algorithm to find prime numbers?, Primality tests)

That's very useful. But how do I stop it from closing the moment it outputs? Is there a setting for that? (I use Microsoft Visual C++ 2010 Express) — Mathime, Mar 12 '16 at 19:31

Prime-testing program not working

1 Answers1