Any advice on cleaning up my program

[smctech]

I have my program but it looks bulky to me.
I may be wrong but here it is tell me what you think.

  // Identify and list the N prime numbers, starting with 2 (2 is a prime number), where N is a number obtained from the user.
// This number N must be in the range of 30 to 2000, inclusive.
// Display the first 20 primes on the screen and save N prime numbers to file "primes.txt"

#include "stdafx.h"
#include <iostream>
#include <iomanip>
#include <fstream>
#include <cmath>

using namespace std;

void primeFunction();
void showResults(int sequence, int& pnum);
void saveResults(int& num);
void fileResults(int sequence, int& pnum);

ofstream outfile;

int main()
{
	// Set data types
	int num = 1;
	int sequence = 1;
	int prime = 1;

	// Get information from user
	cout << "How many prime numbers would you like to see? (0 to quit)" << endl;
	cout << endl;
	cout << "Please enter a whole number between 30 and 2000  > ";
	cin >> num;
	while (num == 0) // Allowing for user to quit program
	{
		cout << endl;
		cout << "Quitting program " << endl;
		cout << endl;
		system("pause");
		return 0;
	}
	while (cin.fail() || num < 30 || num > 2000) // Ensure amount entered is within acceptable range
	{
		cout << "Please enter a whole number between 30 and 2000  > ";
		cin >> num;
		cin.clear();
		cin.ignore(1000, '\n');
		while (num == 0) {
			cout << endl;
			cout << "Quitting program " << endl;
			cout << endl;
			system("pause");
			return 0;
		}
	}
	
	cout << endl;
	cout << endl;
	// Display confirmation message that input is valid and program is ready to proceed
	cout << "Thank you. \nProcessing your results." << endl;
	cout << endl;
	// Clear the screen to prepare for fresh output
	system("pause");
	system("CLS");

	cout << "The first 20 prime numbers are as follows" << endl;
	cout << endl;
	saveResults(num);
	primeFunction();
	cout << endl;
	cout << endl;
	cout << "Please see primes.txt for your full results." << endl;
	cout << endl;

	system("pause");
    return 0;
}

void primeFunction() {
	int n = 1, status = 1, pnum = 2, count, c, sequence = 1;
	for (count = 1; count <= 20; ) //loop that will iterate through n numbers
	{
		for (c = 2; c <= (int)sqrt(pnum); c++)// for each element check whether it is prime or not
		{
			//if number is completely divisible by a number other than 1 and itselft,then number is not prime
			if (pnum%c == 0)
			{
				status = 0;
				break;
			}
		}
		//if it is a prime number, print it
		if (status != 0)
		{
			showResults(sequence++, pnum);
			count++;
		}
		status = 1;
		pnum++;
	}
	return;
}
// Define function for output
void showResults(int sequence, int& pnum) {
	cout << "Prime #" << setw(4) << left << sequence << right << " = " << pnum << endl;
	return;
}
// Define function for output
void fileResults(int sequence, int& pnum) {
	outfile << "Prime #" << setw(4) << left << sequence << right << " = " << pnum << endl;
	return;
}
// Send user requested data to file "primes.txt"
void saveResults(int& num) {
	outfile.open("primes.txt");
	int n = 1, status = 1, pnum = 2, count, c, sequence = 1;
	outfile << "The first " << num << " prime numbers you requested are as follows" << endl;
	outfile << endl;
	for (count = 1; count <= num; ) //loop that will iterate through n numbers
	{
		for (c = 2; c <= (int)sqrt(pnum); c++)// for each element check whether it is prime or not
		{
			//if number is completely divisible by a number other than 1 and itselft,then number is not prime
			if (pnum%c == 0)
			{
				status = 0;
				break;
			}
		}
		//if it is a prime number, print it
		if (status != 0)
		{
			fileResults(sequence++, pnum);
			count++;
		}
		status = 1;
		pnum++;
	}
	outfile.close();

}

Thanks ahead of time

[jlb]

Stop using the global variables and learn to pass the variables into your functions.

You seem to have a lot of code duplication. You should try to reduce this duplication.

For example your fileResults() and showResults() functions are basically the same, if you pass the stream into one of the functions you could eliminate the other.

Your primeFunction() and saveResults() are also almost identical and you could probably combine them in some way.

[TheIdeasMan]

Hi,

The main program asks for a number to calculate primes until a given number, but the rest of it is hard coded to go to 20.

With the algorithm, it is very inefficient to do trial division. Instead consider removing multiples of numbers from a list.

So start out with all the numbers, then remove multiples of 2. The next number in the list is 3, so remove all multiples of that. Now the next is 5, so do the same and continue in that fashion. This is much more efficient, the amount of numbers in the list reduces quickly.

A further enhancement is to use a std::bitset. This is like an array of 0's and 1's, the index is your number and you can go through and set the positions to false if it is a multiple.