printing prime numbers between 1 and N (C++)

[BattleToad]

Perhaps some of you guys who have been doing this longer than I have could help me out. Im writing a console program that reads input from a file (inputdata.txt) and write output to a file (outputdata.txt). The program gets a number from the file, and then outputs all the prime numbers from 1 to the number. My output doesnt seem to add up, the problem is my prime testing function but I cant seem to figure out where. Could someone take a peek at my code and point me in the right direction?

#include <iostream>
#include <fstream>
#include <iomanip>
#include <cmath>

using namespace std;

bool testForPrime(int);
void writeMessage(ofstream &, int);
void writePrimes(ofstream &, int);


int main()
{
	ifstream inputdata;
	ofstream outputdata;
	
	inputdata.open ("a:\\inputdata.txt");
	if (! inputdata)
	{
		cout << "ERROR -- Cannot open inputdata.txt for input" << endl;
		return 1;
	}
	
	outputdata.open ("a:\\outputdata.txt");
	if (! outputdata)
	{
		cout << "ERROR -- Cannot open outputdata.txt for output" << endl;
		return 1;
	}

	int number2Test;

	while (! inputdata.eof())
	{
		inputdata >> number2Test;
		writeMessage(outputdata, number2Test);
		for (int i = 1; i <= number2Test; i++)
		{
			testForPrime(i);
			if (testForPrime)
			{
				writePrimes(outputdata, i);
			}
		}
		outputdata << endl;
	}
}

bool testForPrime(int i)
{
	for (int j = 2; j <= i; j++)
	{
		if (i % j == 0)
		{
			return false;
		}
		else
		{
			return true;
		}
	}
	
}

void writeMessage(ofstream & outputdata, int a)
{
	outputdata << "The number is " << a << "." << " The prime numbers less than or equal to " << a << " are:" << endl;
}

void writePrimes(ofstream & outputdata, int prime)
{
	outputdata << prime << " ";
}

Output with test number of 28

The number is 28. The prime numbers less than or equal to 28 are:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28

[sr71000]

bool testForPrime(int i)
{
	for (int j = 2; j <= i; j++)
	{
		if (i % j == 0)
		{
			return false;
		}
		else
		{
			return true;
		}
	}
	
}

i can see a few problems. First off...if you call j=1 your code will reach the end without returning (error 1)
two...you only make 1 test....if i%j==0 (first call j = 2) is true it returns false else it returns true. It never bothers testing for 3 or 4 or 5 or .... all the way to j! This still isn't your main error though.

	while (! inputdata.eof())
	{
		inputdata >> number2Test;
		writeMessage(outputdata, number2Test);
		for (int i = 1; i <= number2Test; i++)
		{
*************error here******************
			testForPrime(i);
			if (testForPrime)
//should be
//                     if(testForPrime(i));
***************************************
			{
				writePrimes(outputdata, i);
			}
		}
		outputdata << endl;
	}

first you make a method call to testForPrime and it returns a bool. This is never saved anywhere (just thrown away). Then you use testForPrime as a condition for an if statement. I don't even see testForPrime declared as a variable anywhere so i'm kinda surprised this code compiles!

I don't mind helping but that better not have just been your homework assignment or something i just did. lol. Did you write that code or was u'r homework to debug it?

[BattleToad]

oh no I wrote that code, its for a programming assignment that I received last night, and is due like March 24th, I usually just do most of the program while sitting in class when they are assigned, that code is what I wrote last night during my class. I'm definitely against just giving people answers, thats why whenever I need help, I always just ask to be pointed in the right direction, so that I can figure the majority of the problem out on my own, otherwise Ill never learn anything. Thanks for the help though.

oh yeah yeah, using Visual C++ Express 2005, it gives me a warning when I compile that, but it compiles, due to that warning I figured one of the problems was how I wrote that. your way makes much more sense.

[sr71000]

i gave you the bottom one cus it was a minor bug...i take it you're new to programming. Those kinds of mistakes are common while you're learning . The test for prime method you should have a little more fun figuring it out . Sit down and trace through it for each value of j. You'll see what you're doing wrong in no time!

[EXreaction]

Well, I would do something like this for the code...

int num = ; //this would be the number coming in
bool prime = true;
list // I don't remember how to declare an array in C++ anymore, but this needs to be an array :S

for (int i = 1; i <= num; i++)
{
    prime = true;
    for (int j = 1; j <= i; j++)
    {
        if (i%j != 0) // do mod math here to find the remainder, I have done any C++ in a long time or used % but I think it returns 0 if there is no remainder...
        {
            prime = false;
        }
    }

    if (prime == true)
    {
       list[] = i;
    }
}

list // should be the list of prime #'s...

Don't know how well it would work. I haven't done anything in C++ for a long time. But I do quite a bit in php, which is very close to C++.

[jkrohn]

An array needs to be declared as a fixed sized so that it can be allocated onto memory.

As a result list[] = 1 is not a valid operation.

He doesn't need to store them anyway, just write them to the file as he finds them.

Jkrohn

[EXreaction]

Quote:

Originally Posted by jkrohn

An array needs to be declared as a fixed sized so that it can be allocated onto memory.

As a result list[] = 1 is not a valid operation.

He doesn't need to store them anyway, just write them to the file as he finds them.

Jkrohn

I see. Well, you could declare the array as a length of the num coming in...
(sorry, php doesn't have any limitation like that, you can add on to the length whenever you want and I thought it was the same for C++)

Ya, instead of putting them in the list you could just write it to the file there.

[jkrohn]

Quote:

sorry, php doesn't have any limitation like that, you can add on to the length whenever you want and I thought it was the same for C++

That is because php doesn't use actual arrays, rather ordered maps. This comes with a more expensive lookup and insert, but it is easier to use and easier to learn I guess.

Jkrohn

[EXreaction]

I see.

[sr71000]

actually his code is nearly perfect. all he has to do is remove the else return true and tack it on after the for loop. No need for a bool to check conditions. also...as soon as you hit your first i that is a factor of j you should immediately return false (no reason to keep testing for more factors if we already know it's false....it's wasted cycles)