HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Optimizing simple prime number function test

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numbersimplefunctionoptimizingtestprime

Problem

For my first basic project with functions, I wanted to make a program that would allow you to input a number, and it would output whether it was prime or not. For the most part, it's functional and working.

How can I optimize it to be more efficient? How can I make my code more readable for the next person? What are some good programming techniques?

#include "stdafx.h"
#include 
#include 
#include 
#include 
#include 

using namespace std;
int PrimeTest(int x);

int main()
{
    int X = 0;
    cout > X;
    cout << "\n\n\n\t\t\t";
    if (PrimeTest(X) == 1)
    {
        cout << X << " is prime";
    } 
    else
    {

        cout << X << " is NOT prime";
    }

    cout << "\n\n\n\t\t\t";
    return 0;
}

int PrimeTest(int x)
{
    int b = 2;
    while (b < x)
    {
        if (x % b == 0)
        {
            int Answer = 0;
            return Answer;
        }

        b++;
    }
    int Answer = 1;
    return Answer;
}

Solution

-
You have both ` and , but you only need the former. The latter is a C library.

-
You can have
main() at the bottom, allowing you to eliminate the function prototype.

-
Do not use single-character variable names (except for simple loop counters, which can be simple). They are hard to understand for others, and may even leave you confused in the future, especially if you expand on the program quite a bit.

As such,
X should be renamed to something like number.

-
PrimeTest() should return bool, not int. Returning an int is more common in C, but is not done in C++ (it's only done in main(), but it is special). You'll then return either true or false and compare them as such. This will also eliminate the need for Answer.

-
The final output can be simplified to a single-line ternary (
?:) statement:

std::cout << X << " is " << ((PrimeTest(X)) ? "prime" : "NOT prime");


This is essentially a shorter way of writing an
if/else` statement, but is best done with simple cases such as this one. If you'll ever need to expand on these conditions, then it would be better to go back to using curly braces.

Code Snippets

std::cout << X << " is " << ((PrimeTest(X)) ? "prime" : "NOT prime");

Context

StackExchange Code Review Q#66452, answer score: 6

Revisions (0)

No revisions yet.