simple max prime problem

jeraya6 (18)
Hello there. i have this part of a program that i'm wrting.
it's working but it's not giving correct answers. i need to find max prime number of all entered numbers by the user which are betweem 2 and 5 digits.
what am i doing wrong here?
(if u want the whole program i can post it in a reply down)

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
29
30
31
32
 

  cout<<" Enter your numbers then enter -1 to show greatest prime number "<<endl;
    cin>>number;
    if(number<9 || number > 99999)
    {
    cout<<"number is not in the correct range\n";
    exit;
    }
    else
    while(number>9 && number<=99999)
    {
    
    cin>>number;

     for(i = 2;i<=number;i++)
    { 
     if(number%i==0)
     {
         flag=0;
         break;
     }
     if(flag==1)
     {
    if(number>max)    
                     
    max=number;
     }
 
    }
 
    }
    cout<<"the max prime number \n "<< max << endl;
Last edited on
Ganado (6849)
It makes it easier for us to help you if you post full programs that can be compiled without further editing.

First thing is you should work on your indentation. All the text within an if-statement or a loop should be indented. If you have an if statement within a loop, then indent the stuff in the if-statement twice, etc. It helps shows the structure of your program a lot better.

(1) exit is not a special command, it's a function. You need to do something like exit(1); BUT you should really prefer returning something, because exit just abruptly ends the program without doing any cleanup. Prefer, for example, return 1; from main.

(2) You are checking if the flag == 1 every loop. You should only check flag AFTER having iterated through all the necessary numbers to check if a given number is prime.

(3) 9 is a 1-digit number, but won't trigger your range check.
jeraya6 (18)
1- i identify variables in loops in main header , cuz i use them in more than 1 section
-2 i use exit just to break .. it's not a big deal now but thanks for the heads up
3-i check flag cuz i have a set of numbers entered by the user and i need to check if it's prime or not THEN check max prime
4- since the program allows 2-digit to 5-digit numbers i use this while .. that's it

i'll post full program now
jeraya6 (18)
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
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
 
#include <iostream>  
#include <math.h>   
using namespace std;  

int main()
{
    int  terms , i, k, fact, signChanger=-1, choose, student, Nmark, mark, flag=1, number,count=0, max=0;
    float x , pwr, sum=0, avg=0, passAvg=0 ;
    cout<<"  1- Print number of failed students and average marks of passed students.\n  2- Print max prime number. \n  ";
    cout<<"3- print out the result of sin(x) \n  4- exit \n";
    cin>>choose;
    
    if(choose==1)
    {
    cout << " How many students " <<endl;
    cin >>student;
    for (i = 1 ;i <= student; i++)
    {
    cout << "How many marks " << endl;
    cout << "Please enter the same number of marks for all students " <<endl; // to get a reasonable avarege
    cin >> Nmark;
    for (int a=1; a <= Nmark; a++)
    {
    cout << "enter the mark" << endl;
    cin >> mark; 
 
    sum= sum + mark;
 
    avg=sum/Nmark;
    }
    if (avg > 60)
    passAvg= passAvg + sum/Nmark;
    else
    count ++;
    sum = 0;
    }
    cout <<"the avarege for passed students is : " <<passAvg<< endl;
  
    cout <<"the number of failed students is : "<<count<< endl;  
    }
    
    else
   if (choose==2)
    {
    cout<<" Enter positive numbers then enter -1 to show greatest prime number "<<endl;
    cin>>number;
    if(number<9 || number > 99999)
    {
    cout<<"number is not in the correct range\n";
    return 1;
    }
    else
    while(number>9 && number<=99999)
    {
    if(number < 0)
    cout<<"enter only positive numbers ";
    cin>>number;

     for(i = 2;i<=number;i++)
    { 
     if(number%i==0)
     {
         flag=0;
         break;
     }
     else if(flag==1)
     {
    if(number>max)    
                     
    max=number;
     }
 
    }
 
    }
    cout<<"the max prime number \n "<< max << endl;
    }
    
    else
    if (choose==3)
    {
	cout<<"enter the value of x "<<endl; 
	cin>>x;
	cout<<"  enter the value of n terms "<<endl; 
	cin>>terms; 
	
	for(i=1 ; i<=terms; i=i+2 )
	{
	pwr=1; 
    fact= 1;
    for(k=1; k<=1 ; k++)
    {
		pwr=pwr*x;  
			
		fact=fact*k; 
			
	} 
	    signChanger= signChanger * -1;
		sum = sum+ signChanger*pwr /fact; 
	} 
	

	
	cout<<" Sum is :"<<sin(sum)<<endl;
   }
   else 
   if (choose==4)  //exits when user presses 4
   {
     return 1;
   }
   else
   return 1;  // exits if user presses a number or character other than the given options
   
    return 0;
}
jlb (4973)
You really need to work on your indentation. Your inconsistent "style" makes reading your program difficult.

Next, you really should start declaring your variables close to first use instead of one big glob at the beginning of some scope. This includes declaring and initializing your for() loops in the initialization section of the loop.

Next I recommend you start getting used to the idea of zero based for() loops so that when you get to vectors/arrays you won't have problems with overflowing the vector/array by one.

1
2
3
4
5
6
7
8
9
 
for(int a = 0; a < Nmark; a++) 
{
    cout << "enter the mark" << endl;
    cin >> mark;
    
    sum += mark;
    
    avg = sum / Nmark;
}


Also functions should be your friends, when properly used they will greatly simplify your program flow.

I'd also recommend you use braces with all of your control statements, even those that technically don't require them. Always using the braces is never wrong, but not using the braces can cause hard to find logic bugs.

Lastly, for now, you should strive to separate the user interface from the program logic whenever possible.

jeraya6 (18)
Hey thanks for the recommendations, i'm still trying to solve the problem i came here for .. not grading my style :\ but thanks
jlb (4973)
IMO, your lack of style is probably a big part of the "problem". Your program is so hard to read that you're probably missing simple problems.


Repeater (3046)
Your "style" is making it actively harder to understand your code. It's not "style"; it's bad programming.

This is the code in your program that's looking for prime numbers:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
     for(i = 2;i<=number;i++)
		{ 
		  if(number%i==0)
		    {
		      flag=0;
		      break;
		    }
		  else if(flag==1)
		    {
		      if(number>max)    
                     
			max=number;
		    }
		}
	    }
	cout<<"the max prime number \n "<< max << endl;


What does this do? It doesn't make any sense as a prime number finder. Max starts at zero... and then you just set max to be the same as number ... and you stop when you find a number that is a divisor of the target number, and you return max (which is sometimes still zero).

Flag is serving no purpose at all here.

This code just sets max to number, and that's it.

This just makes no sense.

Here's a complete program showing your function:

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
29
30
31
 
#include <iostream>

using namespace std;
int function(int number)
{
  int max = 0;
  for(int i = 2;i<=number;i++)
    { 
      if(number%i==0)
	{
	  
	  break;
	}
      else 
	{
	  if(number>max)    
                     
	    max=number;
	}
    }

  cout<<"the max prime number (from input "<< number << ") : " << max << endl;
}

int main()
{
  for (int x = 3; x < 100; ++x)
    {
      function(x);
    }
}


Run it. See what your code is doing. Write code that actually finds prime numbers.

Here's some sample output:
the max prime number (from input 3) : 3
the max prime number (from input 4) : 0
the max prime number (from input 5) : 5
the max prime number (from input 6) : 0
the max prime number (from input 7) : 7
the max prime number (from input 8) : 0
the max prime number (from input 9) : 9
the max prime number (from input 10) : 0
the max prime number (from input 11) : 11

See what it does? Nothing to do with finding prime numbers.
Last edited on
dhayden (5799)
i'm still trying to solve the problem i came here for .. not grading my style

Consistent indentation will help you find the bugs. Put another way, if you can't easily see the block structure by looking at the code, you're practically guaranteed to screw it up. As an aside, the Python language takes this to an extreme: the indentation is the block structure - there are no {} or begin/end keywords.

Here is your code with consistent indentation:
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
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
 
#include <iostream>
#include <math.h>
using namespace std;

int
main()
{
    int terms, i, k, fact, signChanger = -1, choose, student, Nmark, mark,
	flag = 1, number, count = 0, max = 0;
    float x, pwr, sum = 0, avg = 0, passAvg = 0;
    cout << "  1- Print number of failed students and average marks of passed students.\n  2- Print max prime number. \n  ";
    cout << "3- print out the result of sin(x) \n  4- exit \n";
    cin >> choose;

    if (choose == 1) {
	cout << " How many students " << endl;
	cin >> student;
	for (i = 1; i <= student; i++) {
	    cout << "How many marks " << endl;
	    cout << "Please enter the same number of marks for all students " << endl;	// to get a reasonable avarege
	    cin >> Nmark;
	    for (int a = 1; a <= Nmark; a++) {
		cout << "enter the mark" << endl;
		cin >> mark;

		sum = sum + mark;

		avg = sum / Nmark;
	    }
	    if (avg > 60)
		passAvg = passAvg + sum / Nmark;
	    else
		count++;
	    sum = 0;
	}
	cout << "the avarege for passed students is : " << passAvg << endl;

	cout << "the number of failed students is : " << count << endl;
    }

    else if (choose == 2) {
	cout << " Enter positive numbers then enter -1 to show greatest prime number " <<
	    endl;
	cin >> number;
	if (number < 9 || number > 99999) {
	    cout << "number is not in the correct range\n";
	    return 1;
	} else
	    while (number > 9 && number <= 99999) {
		if (number < 0)
		    cout << "enter only positive numbers ";
		cin >> number;

		for (i = 2; i <= number; i++) {
		    if (number % i == 0) {
			flag = 0;
			break;
		    } else if (flag == 1) {
			if (number > max)

			    max = number;
		    }

		}

	    }
	cout << "the max prime number \n " << max << endl;
    }

    else if (choose == 3) {
	cout << "enter the value of x " << endl;
	cin >> x;
	cout << "  enter the value of n terms " << endl;
	cin >> terms;

	for (i = 1; i <= terms; i = i + 2) {
	    pwr = 1;
	    fact = 1;
	    for (k = 1; k <= 1; k++) {
		pwr = pwr * x;

		fact = fact * k;

	    }
	    signChanger = signChanger * -1;
	    sum = sum + signChanger * pwr / fact;
	}

	cout << " Sum is :" << sin(sum) << endl;
    } else if (choose == 4) {			 //exits when user presses 4
	return 1;
    } else
	return 1;				 // exits if user presses a number or character other than the given options

    return 0;
}


Indented this way, some problems leap to my eyes:
- Line 56 sets flag=0 and it's never reset to 1. Since flag indicates whether number is prime, it should be set to 1 before you start checking for primality. If you had defined flag closer to where it's used, you might not have had this bug.

- Lines 58-62 are inside the for loop, so you're setting max before you even know if number is prime. I suspect that you thought this code was outside the loop, where it belongs. So here's a bug that was hidden by inconsistent indentation.

- Line 42 prompts for a number which is entered at line 44. But if you enter a valid number, line 52 forces you to enter it again, this time without any sort of prompt. As a result, the first number entered is never checked. If it turns out to be the maximum prime, the output will be incorrect.

The program exits at line 91 or 93. There is no loop, for more input. Again, you can see this from the indentation.

Have you learned about functions? It would be easier to keep the logic straight if you used functions for this program.
jeraya6 (18)
hey people i fixed it ...
else if (flag == 1) i turned this if statement in to else if (flag)
can some1 explain why removing "==1" fixes the problem ? thanks
keskiverto (10425)
cout<<" Enter positive numbers then enter -1 to show greatest prime number "<<endl;

What does that actually mean?

To me it looks like:
max = 0
user gives some numbers and then -1
FOR EACH number
  IF number == -1 THEN break loop
  IF number is in valid range AND number is prime AND number > max
  THEN max = number

print max

This logic will print 0, if no number from user is a prime.
Repeater (3046)
Given that in your code flag is only 0 or 1, this

else if (flag == 1)

is effectively identical to this

else if (flag)

and makes no difference.
Topic archived. No new replies allowed.