Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is my first C++ program and I was wondering if I'm using too many variables. Its a simple program that gets a number from a user and calculates the number of primes between 3 and the user's number. I'm just curious if i'm using too many variables. I'm only asking since, I'm used to not needing to initialize iterator variables before a loop.

#include <iostream>
using namespace std;
int isPrime(int);
int main() {
    int userValue;
    int b;
    int x;
    char c;
    cout<< "Enter a number : "<< flush;
    cin >> userValue;
    for (x = 3; x <= userValue; x= x+2){
        a = isPrime(x);
        if (a == 1){ 
        b +=1;    
        }
    }
    cout << "There is, " << b << " number of primes from 3 to "<< userValue << endl;    
    cout << "hit enter to terminate..."<< flush;
    cin.get();
    cin.get();    
}

int isPrime(int number){
    int i;
    for(i=2; i<number; i++){
    if (number % i == 0 && i != number) return 0;
    }
    return 1;
}
share|improve this question
1  
You didn't ask for this but you could test if an integer is a prime in a much more efficient way. –  Josay Feb 4 '13 at 2:35
add comment

1 Answer

up vote 2 down vote accepted

No, I wouldn't say you're using too many variables (except for c, which doesn't appear to be used anywhere). Not enough functions, though. And perhaps too many spurious assignments. Specifically, I'd suggest something like:

#include <iostream>
using namespace std;

bool isPrime(int);
int countPrimes(int);
int main() {
    int userValue;
    cout<< "Enter a number : "<< flush;
    cin >> userValue;

    cout << "There are, " << countPrimes(userValue) << " number of primes from 3 to "<< userValue << endl;    
    cout << "hit enter to terminate..."<< flush;
    cin.get();
    cin.get();    
}

int countPrimes(int number) {
    int result = 0;
    for (int x = 3; x <= number; x += 2){
        if (isPrime(x)){ 
            result++;    
        }
    }
    return result;
}

bool isPrime(int number){
    for(int i=2; i<number; i++){
        if (number % i == 0 && i != number) return false;
    }
    return true;
}

That will do the same thing as your code. Also, please consider using variable names that are not just single-letters.

share|improve this answer
    
Thanks for catching the unused c variable. I converted this program from C program which needed a character variable for a scanf() function. –  Austin Davis Feb 4 '13 at 4:21
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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