Unexpected output with code to finding the largest prime factor of a large number

Hello All.
I was trying to find out the highest prime factor of a very large number(600851475143). I wrote the code as follows:
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
  #include <cstdlib>
#include <iostream>
#include <fstream>
#include <cmath>
using namespace std;

int check(unsigned long long x)
{
	for(unsigned long long y=(--x);y>0;--y)
	{
		if(y==1 && x%y==0)
		{
			return 1;
		}
		else if(x%y!=0 && y!=1)
		{
			continue;
		}
		else
		{
			return 0;
		}
	}
}
int main()
{
    system("cls")
    /*Program to calculate the highest prime factor of given digit*/;
    unsigned long long srt, num=600851475143;
	srt=num;
	for(unsigned long long x=srt;x<0;--x)
	{
		if(num%x==0)
		{
			if(check(x)==1)
			{
				cout<<x;
				break;
			}
			else
			{
				continue;
			}
		}
		else
		{
			continue;
		}
	}
	cout<<"Program made by TheSherlockHolmie";
	cout<<endl;
    return EXIT_SUCCESS;
}

As can be seen, I used a for loop to find the factors of the number. To check if the factor obtained is prime or not, I am using a function check(), i made. I was expecting the output to give out the factor, but the only thing that appears is "Program made by TheSherlockHomie". I am using Dev C++ 5.8.2 on gcc 4.8.1 64 bit release. Pointers on the code will be gratefully appreciated.
Last edited on
please explain your check() function
for(unsigned long long y=(--x);y>0;--y)
Do you really need to decrease x here? Remember, decrement change value of operand, so your code is equivalent to:
1
2
x = x-1;
for(unsigned long long y= x; y>0; --y)
If you do not output your result it will not appear magically.

1
2
cout<<"Program made by TheSherlockHolmie";
	cout<<endl;


That will only be your only output...
Line 37 outputs value after checking.
Thank you all for the response.
@MiiNiPaa, Thanks for pointing that decrement point out. So the new code will be:
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
#include <cstdlib>
#include <iostream>
#include <fstream>
#include <cmath>
using namespace std;

int check(unsigned long long x)
{
	for(unsigned long long y=(x-1);y>0;--y)
	{
		if(y==1 && x%y==0)
		{
			return 1;
		}
		else if(x%y!=0 && y!=1)
		{
			continue;
		}
		else
		{
			return 0;
		}
	}
}
int main()
{
    system("cls")
    /*Program to calculate the highest prime factor of given digit*/;
    unsigned long long srt, num=600851475143;
	srt=num;
	for(unsigned long long x=srt;x<0;--x)
	{
		if(num%x==0)
		{
			if(check(x)==1)
			{
				cout<<x;
				break;
			}
			else
			{
				continue;
			}
		}
		else
		{
			continue;
		}
	}
	cout<<"Program made by TheSherlockHolmie";
	cout<<endl;
    return EXIT_SUCCESS;
}

But even after this, there is no output.
@ne555, the check() function checks if the number is prime by dividing it with every value below it upto 1,and checking the remainder(x%1 does not count). If the number is prime, it returns value 1, and if it is not, returns 0.
@Delshire, the program does have a cout in line 37, as @MiiNiPaa said.
for(unsigned long long x=srt;x<0;--x)

How many times do you think your loop will be executed.
I decided to not wait until you run into next problem. This is your code devoid of all exceess and unneeded parts, slightly refactored.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
#include <iostream>
#include <cmath>

bool is_prime(unsigned long long x)
{
    unsigned long long threshold = std::sqrt(x) + 1;
    for(unsigned long long y = 1; y < threshold; ++y)
        if(x%y==0) return false;
    return true;
}
int main()
{
    unsigned long long num=600851475143;
    for(unsigned long long x = num - 1; x > 1; --x)
        if(num%x == 0 && is_prime(x)) {
            std::cout << x;
            break;
        }
    std::cout<<"Program made by TheSherlockHolmie\n";
}
It is expected to run on i5-2400 in just under an hour. Because your algorithm is quadratic and unefficient and your number is large.

Straightforward approach with finding prime factorisation runs in under a second.

TL;DR: do not waste time fixing this code, it is too slow. Choose different approach.
1
2
3
4
5
6
7
8
9
#include <iostream>
using namespace std;
int main()
{
unsigned long long x= 600851475143;
for(unsigned long long y=2;y<x;++y)
if(x%y==0){--x;y=2;}
cout<<x;
}


It may take a while, because the number is so large. Not sure if the reason you were making it complex, so it would be faster? But i'm still new =p
It does not work. At all. I am not sure what it is supposed to do.
@MiiniPaa, thanks for your feedback. And thanks for the code, but it is not running. What approach/algorithm do you suggest to run this thing? Give me a hint, because I really want to do this myself, being a beginner notwithstanding.
@DyavolskiMost, yeah, it will. It was given to me as a problem, and I am required to give the answer. Luckily, there is no time limit, but I really want to solve it. I know that there is a very efficient algorithm that i am not able to think of.

Thanks all.
P.S. I am going on a holiday, so might take a bit long to reply to the next post.
but it is not running
It should run as it is essentually your code without excess parts and slightly optimised. It just takes ungodly amount of time to finish.

I suggest you to do prime number factorisation: find first divisor of the number (it will be prime by definition), then divide number by it until it cannot be divided anymore. Then find next divisor...

Last found divisor will be largest prime divisor for this number. Alternatively you can save all divisor in vector to do something with them
Topic archived. No new replies allowed.