using namespace std
Please see: Why is "using namespace std;" considered bad practice?
Naming
PrimeNumber
This class doesn't deserve this name, because it doesn't represent a prime number. It represents a big big number of which you want to retrieve the prime factors. This leads to the getVector()
method which should be better named getPrimeFactors()
. In addition to this, vector<PrimeNumber> primes;
should be renamed also.
getPrimes()
This method does not get primes, but prime factors, so a rename should be done to e.g getPrimeFactors()
.
Comments
Comments should be comment on why is something done instead of what is done. See
//return the finished vector return vec;
Because comments can become misleading fast, use them with care and update them also if your code changes. See
vector<long long> temp1; // empty vector vector<long long> result1; // temp vector
In addition, if you need a comment to describe what a variable is for, or what it should represent, then you didn't name your variable good.
Single responsibility principle
Your main()
method does a way to much.
- processing input from user
- calling the calculation
- printing the results
- measuring the time
You should extract some of these to separate methods.
What I really like: You are using braces {}
for single if
statements also.
Algorithm
To speed up your algorithm you should
- precalculate an amount of prime numbers up to a defined border, by using e.g Sieve of Eratosthenes.
- Check if these primen umbers are sufficient to get all prime factors.
- If they are not sufficient, get the next prime number, store it and go back to
2.
to check, if thenew
prime number is a prime factor of your number.
- 50.9k
- 5
- 83
- 177