Overwiev of the program?

I made a program that can tell you if a number is a prime number or not.
Here is the code:

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
#include "stdafx.h"
#include <iostream>
#include <math.h>
using namespace std;

float x;
float a;
unsigned short int praštevilo [] = { 2, 3,	5,	7,	11,	13,	17,	19,	23,	29,	31,	37,	41,	43,	47,	53,	59,	61,	67,	71,
73,	79,	83,	89,	97,	101,	103,	107,	109,	113,	127,	131,	137,	139,	149,	151,	157,	163,	167, 173,
179,	181,	191,	193,	197,	199,	211,	223,	227,	229,	233,	239,	241,	251,	257,	263,	269,	271,	277,	281,
283,	293,	307,	311,	313,	317,	331,	337,	347,	349,	353,	359,	367,	373,	379,	383,	389,	397,	401,	409,
419,	421,	431,	433,	439,	443,	449,	457,	461,	463,	467,	479,	487,	491,	499,	503,	509,	521,	523,	541,
547,	557,	563,	569,	571,	577,	587,	593,	599,	601,	607,	613,	617,	619,	631,	641,	643,	647,	653,	659,
661,	673,	677,	683,	691,	701,	709,	719,	727,	733,	739,	743,	751,	757,	761,	769,	773,	787,	797,	809,
811,	821,	823,	827,	829,	839,	853,	857,	859,	863,	877,	881,	883,	887,	907,	911,	919, 929,	937,	941,
947,	953,	967,	971,	977,	983,	991, 997, 1009, 1013, 1019, 1021, 1031, 1033, 1039, 1049, 1051, 1061, 1063, 1069,
1087, 1091, 1093, 1097,	1103, 1109, 1117, 1123,	1129, 1151, 1153, 1163,	1171, 1181,	1187,	1193,	1201,	1213,	1217,	1223, 1229,	1231,	1237,	1249,
1259,	1277,	1279,	1283,	1289,	1291,	1297,	1301,	1303,	1307,	1319,	1321,	1327,	1361,	1367,	1373,
1381, 1399, 1409, 1423,	1427, 1429, 1433, 1439,	1447, 1451, 1453, 1459,	1471, 1481,	1483,	1487,	1489,	1493,	1499,	1511,
1523, 1531, 1543, 1549,	1553, 1559, 1567, 1571,	1579, 1583, 1597, 1601,	1607, 1609,	1613,	1619,	1621,	1627,	1637,	1657,
1663, 1667, 1669, 1693,	1697, 1699, 1709, 1721,	1723, 1733, 1741, 1747,	1753, 1759,	1777,	1783,	1787,	1789,	1801,	1811,
1823, 1831, 1847, 1861,	1867, 1871, 1873, 1877,	1879, 1889, 1901, 1907,	1913,	1931,	1933,	1949,	1951,	1973,	1979,	1987,
1993, 1997, 1999, 2003,	2011, 2017, 2027, 2029,	2039, 2053, 2063, 2069,	2081,	2083,	2087,	2089,	2099,	2111,	2113,	2129,
2131, 2137, 2141, 2143,	2153, 2161, 2179, 2203,	2207, 2213, 2221, 2237,	2239,	2243,	2251,	2267,	2269,	2273,	2281,	2287,
2293, 2297, 2309, 2311,	2333, 2339, 2341, 2347,	2351, 2357, 2371, 2377,	2381,	2383,	2389,	2393,	2399,	2411,	2417,	2423,
2437, 2441, 2447, 2459,	2467, 2473, 2477, 2503,	2521, 2531, 2539, 2543,	2549,	2551,	2557,	2579,	2591,	2593,	2609,	2617,
2621, 2633, 2647, 2657,	2659, 2663, 2671, 2677,	2683, 2687, 2689, 2693,	2699,	2707,	2711,	2713,	2719,	2729,	2731,	2741,
2749, 2753, 2767, 2777,	2789, 2791, 2797, 2801,	2803, 2819, 2833, 2837,	2843,	2851,	2857,	2861,	2879,	2887,	2897,	2903,
2909, 2917, 2927, 2939,	2953, 2957, 2963, 2969,	2971, 2999, 3001, 3011,	3019,	3023,	3037,	3041,	3049,	3061,	3067,	3079,
3083, 3089, 3109, 3119,	3121, 3137, 3163, 3167,	3169, 3181, 3187, 3191,	3203,	3209,	3217,	3221,	3229,	3251,	3253,	3257,
3259, 3271, 3299, 3301,	3307, 3313, 3319, 3323,	3329, 3331, 3343, 3347,	3359,	3361,	3371,	3373,	3389,	3391,	3407,	3413,
3433, 3449, 3457, 3461,	3463, 3467, 3469, 3491,	3499, 3511, 3517, 3527,	3529,	3533,	3539,	3541,	3547,	3557,	3559,	3571 };

int main()
{ 
 cout <<"Enter an integer bigger than 1" <<endl;
 cin >> x;
 unsigned short int y = 0;
 float o = x/2;
 skok:
  if ( praštevilo [y] > o)	  
   cout << x <<" is a prime number." << endl;
  else if ( praštevilo [y] <= o)
  {
	  	 a = praštevilo [y];
	 float r = x / a ;
	 float b = ceil(r) ;
	 float k = b - r;
	 if ( k == 0)
	  cout << x <<" isn't a prime number"<< endl;
	 else if ( k != 0)
	  {
	   y = y + 1 ;
	   goto skok;
	  }
  }
cin.get();
cin.get();
return 0;
}


I would be happy if anyone could tell me how to make my code more efficient (any work around for goto part?) and should I add any extra features for practice ( I plan on adding the 0 and 1 exeption tommorrow).
Thank you very much.
Last edited on
This isn't a valid identifier in C/++: praštevilo
Here are the constraints for identifiers in C/++: Only Latin alphabet characters (A-Z and a-z), underscores (_), and Arabic numerals anywhere but as the first character (e.g. '_1id' is valid, but '1id' isn't).

I'm sorry to say this, but a primality test must be the most retarded use for a lookup table I've seen in my life.
A combination of bad variable names, weird logic, and goto-looping makes it very hard to understand what your code is doing.

I'm going to say this to you now, so you don't have to hear it in a professional environment: never, EVER, loop with goto. There's no way I can stress this enough.
Goto still has a few legitimate uses, but looping is under no circumstance one of them. This isn't the 1950s. We now have structures that relieve us from the burden of manually writing our own loops. Furthermore, manually writing structures with goto has a high danger of producing spaghetti code.


Now, about primality test: the fastest naïve primality test algorithm involves checking divisibility of n by all integers in the range [2;n^.5].
http://en.wikipedia.org/wiki/Primality_test
Last edited on
Hmmmm weird praštevilo worked fine for me.
Also mind I am only 13 and it's my first program.
And I am using the naïve primality test but I am just using the prime numbers to check if they are divisible (no need to check for compsite numbers as they are divisible by primes.) and that is why I use the array of prime numbers.
Thanks for feedback anyway.
Last edited on
Also you can initialize the array praštevilo in a better way,with a loop.Take all the numbers from eg 0-2500 and check if it is a prime number and then put it on an array(praštevilo on our example).If I were you I wouldnt initialize the array on your way.
Good luck.
Well I remade the code.
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
#include "stdafx.h"
#include <iostream>
#include <math.h>
using namespace std;

unsigned long int y;
float x;
float r;
float k;
float b;
float m;

int main()
{ 
do { cout <<"Enter an integer bigger than 1 (0 if you want to exit)."<< endl;
cin >> y;
m =  y/2;
x = ceil(m);
do
 {r = y/x ;
  k = ceil(r);
  b = k - r;
  if ( b == 0 )
  {
  cout << y <<" isn't a prime number. It is divisible by "<< r <<"."<< endl;
  break;
  }
  else
  x--;
} while ( x>1);
if (x>1)
cout << endl;
else
cout << y <<" is a prime number."<< endl;
} while (y!=0);
return 0;
}


My question is is this any better then my last code?
And if it is how could I make it better?
Thank you in advance.
Last edited on
I am pretty sure, if you used unsigned integers instead of floating point numbers then there would be no need to use the ceil function to round up, hence you would need to declare less variables & use less functions.

When you use "m = y/2" & then "x = ceil(m)" you could just do:
m = ceil(y/2);
Hence, using another less variable, fewer lines of code.

Also as a point: as soon as you get the number input, check if it is disivible by 2, this way, instead of using "x--" you would be able to get it to "x-=2" & thus have to check your input against approximately half as many numbers, good for if you enter a large number.

I am sure other people can add to my list =)
In the operation y/2 you are dividing two integers and the result will be a truncated integer, in order to have a precise result change 2 in 2.0 so it will have double as type
Put the primality test code in a function with this prototype:
bool isprime(unsigned long n);
If your compiler supports it, you can even use unsigned long long.

I agree. You should only need and use integral types. It will make your code faster for two reasons: 1) operations on integers are faster than operations of floats, and 2) no reason to call ceil().
Avoid calling any of the functions defined in cmath from inside a loop. They are VERY slow. For example, instead of calling abs(), define a macro that does the same, like this one:
#define ABS(x) ((x)<=0?-(x):(x))
That macro is many times faster than abs().

EDIT:
In the operation y/2 you are dividing two integers and the result will be a truncated integer

That's true. I didn't notice that. That line right there is performing a useless integer-to-floating-point conversion, which are also very slow.
Last edited on
Topic archived. No new replies allowed.