Skip to main content
Code Review

Return to Revisions

2 of 4
added 407 characters in body
Heslacher
  • 50.9k
  • 5
  • 83
  • 177

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

  1. precalculate an amount of prime numbers up to a defined border, by using e.g Sieve of Eratosthenes.
  2. Check if these primen umbers are sufficient to get all prime factors.
  3. If they are not sufficient, get the next prime number, store it and go back to 2. to check, if the new prime number is a prime factor of your number.
Heslacher
  • 50.9k
  • 5
  • 83
  • 177
default

AltStyle によって変換されたページ (->オリジナル) /