B04f7f475867f6b47a59b49dfabc0daf

This is a simple program to determine whether or not a number is prime. I'm wondering if I could make the prime determination part more streamlined (the stuff inside the for loop). Also, the last part where it asks them to hit enter doesn't seem to be working. I'd like for it to pause until they hit any key, clear the screen, then start all over again.

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
#include <iostream.h>
#include <stdlib.h>
#include <math.h>

int main() {
	
	while(true) {

		int num;
		
		cout << "Give me a positive integer: ";
		cin >> num;
		
		while (num < 1 ) {
			cout << "Please enter a POSITIVE number: ";
			cin >> num;
		}
		
		int p = 0;

		for ( int i = 2; i <= sqrt(num); i++ ) {

			if ( num % i == 0 ) {
				p = 1;
			}

			if (p == 1 )
				break;
		}

		cout << num << (p == 1 ? " is NOT prime." : " IS prime.") << endl;
		
		cout << "\n\nHit ENTER to try another one";
		system("cls");
		
	}
	
	return 0;
}

Refactorings

No refactoring yet !

A8d3f35baafdaea851914b17dae9e1fc

Adam

October 8, 2008, October 08, 2008 15:50, permalink

No rating. Login to rate!

A refactoring with basic I/O. Use a library like ncurses if you want to be able to clear the screen, wait for user input, etc.

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
#include <iostream>
#include <openssl/bn.h>

class Bignum
{
public:
    Bignum(unsigned long number)
    {
        big_number = BN_new();
        BN_set_word(big_number, number);
    }
    
    ~Bignum()
    {
        BN_free(big_number);
    }
    
    int is_prime()
    {
        return BN_is_prime(big_number, 0, NULL, NULL, NULL);
    }
    
protected:
    BIGNUM *big_number;
};

int main(int argc, char **argv)
{
    unsigned long prime;
    const char *messages[] = { " is NOT prime.", " IS prime." };
    
    while (true) {
        std::cout << "Give me a positive integer: ";
        std::cin  >> prime;
  
        std::cout << prime
                  << messages[Bignum(prime).is_prime()]
                  << std::endl;
    }
    
    return 0;
}
B04f7f475867f6b47a59b49dfabc0daf

joshuamc

October 8, 2008, October 08, 2008 16:19, permalink

No rating. Login to rate!

This is beyond my knowledge. I'd like to just try and do it myself with a for loop.

A8d3f35baafdaea851914b17dae9e1fc

Adam

October 8, 2008, October 08, 2008 20:41, permalink

No rating. Login to rate!

Just playing around with a ncurses version.

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
#include <sstream>
#include <curses.h>
#include <openssl/bn.h>

class Bignum
{
public:
    Bignum(unsigned long number)
    {
        big_number = BN_new();
        BN_set_word(big_number, number);
    }
    
    ~Bignum()
    {
        BN_free(big_number);
    }
    
    int is_prime()
    {
        return BN_is_prime(big_number, 0, NULL, NULL, NULL);
    }
    
protected:
    BIGNUM *big_number;
};

class Screen
{
public:
    Screen()
    {
        initscr();
    }
    
    ~Screen()
    {
        endwin();
    }
    
    void write(const char *string)
    {
        addstr(string);
    }
    
    void write(const char *string, int y, int x)
    {
        move(y, x);
        write(string);
    }
    
    void write(std::string string)
    {
        write(string.c_str());
    }
    
    void write(std::string string, int y, int x)
    {
        write(string.c_str(), y, x);
    }
    
    void read(unsigned long *value)
    {
        scanw("%lu", value);
    }
    
    void wait()
    {
        getch();
    }
    
    void clear()
    {
        ::clear();
    }
};

int main(int argc, char **argv)
{
    Screen screen;
    unsigned long prime;
    const char *messages[] = { " is NOT prime.", " IS prime." };
    
    while (true) {
        screen.clear();
        screen.write("Give me a positive integer: ");
        screen.read(&prime);
        
        std::stringstream message;
        message << prime
                << messages[Bignum(prime).is_prime()]
                << std::endl;
        
        screen.write(message.str(), 1, 0);
        screen.write("Press any key to try another. ", 3, 0);
        screen.wait();
    }
    
    return 0;
}
3d920d12e54fc518dcacabbccb2e6138

Chris

October 15, 2008, October 15, 2008 00:37, permalink

No rating. Login to rate!

This is 2 easy.

B04f7f475867f6b47a59b49dfabc0daf

goodespeler.myopenid.com

October 15, 2008, October 15, 2008 11:10, permalink

No rating. Login to rate!

This assignment has come to pass, but the we were only allowed to use a while loop, for loop, if/else, and the libraries I included.

Avatar

santa

October 18, 2008, October 18, 2008 18:03, permalink

No rating. Login to rate!

since it's ran on multiple big nums, adam, why not store a list of primes encountered by then and only check vs them and normal numbers above them (updating the list) - this would complicate the process but would save time when ran on big numbers... <- of course computers aren't as weak today but had it had to run on large numbers and in quantities...

A8d3f35baafdaea851914b17dae9e1fc

Adam

October 20, 2008, October 20, 2008 01:44, permalink

No rating. Login to rate!

@santa

1. Quality of code is more important than performance, *unless* a bottleneck is identified. For my purposes, the implementation was more than adequate performance wise. No need to degrade the code any further with added complication.

2. I'm not exactly keen on the BigNum, but it was a necessary evil to utilize the prime function of the OpenSSL library. There is no point in reinventing the wheel here; using an existing implementation is a no brainer. I could have evaluated another library that operated on floats directly, and probably would have for my own code. But for the purposes of refactormycode I wanted to use a library that was relatively ubiquitous. OpenSSL fit that bill.

Your refactoring





Format Copy from initial code

or Cancel