patterncppMinor
Remake of factoring/prime factorization calculator
Viewed 0 times
factoringremakeprimecalculatorfactorization
Problem
Background info on factoring and prime factorization.
I've remade this program from my first attempt, and I'm sure it can still be improved. My questions:
Factors.h
Factors.cpp
Primes.h
Primes.cpp
```
#include "Primes.h"
#include
Primes::Primes(std::uint64_t i) : integer(i) {}
Primes::PrimesList Primes::calculate() const
{
std::uint64_t intCopy = integer;
std::uint64_t divisor = 2;
PrimesList primes;
while (intCopy % divisor == 0)
{
intCopy /= divisor;
primes[divis
I've remade this program from my first attempt, and I'm sure it can still be improved. My questions:
- These calculations should only be done with positive integers. Would something like
templates help keep this type-safe? What about throwing an exception?
- Is this an effective way to use the
typedefs? Only the classes need them, so it seems better to qualify them with each class rather than anamespace.
- Can both
calculate()s be better optimized, considering they perform numerous divisions?
- I'm not quite liking the call to
calculate()in bothdisplay()s, even though there are no data member changes. Is this still okay? It doesn't quite make sense to perform the calculations in the constructor, andcalculate()should still beprivate.
Factors.h
#ifndef FACTORS_H
#define FACTORS_H
#include
#include
class Factors
{
private:
typedef std::map FactorsList;
std::uint64_t integer;
FactorsList calculate() const;
public:
Factors(std::uint64_t);
void display() const;
};
#endifFactors.cpp
#include "Factors.h"
#include
#include
Factors::Factors(std::uint64_t i) : integer(i) {}
Factors::FactorsList Factors::calculate() const
{
float sqrtInt = std::sqrt(static_cast(integer));
FactorsList factors;
for (std::uint64_t i = 1; i first second << "\n";
}
}Primes.h
#ifndef PRIMES_H
#define PRIMES_H
#include
#include
class Primes
{
private:
typedef std::map PrimesList;
std::uint64_t integer;
PrimesList calculate() const;
public:
Primes(std::uint64_t);
void display() const;
};
#endifPrimes.cpp
```
#include "Primes.h"
#include
Primes::Primes(std::uint64_t i) : integer(i) {}
Primes::PrimesList Primes::calculate() const
{
std::uint64_t intCopy = integer;
std::uint64_t divisor = 2;
PrimesList primes;
while (intCopy % divisor == 0)
{
intCopy /= divisor;
primes[divis
Solution
Factors::calculate() can be sped up by:-
performing two integer adds after each iteration to determine loop termination
std::sqrt()'s overhead, calculations, and casting can be slow
for-loops shouldn't increment towards a floating-point value
- incrementing towards an integer value is faster and proper
- adding is generally fast and can be done in place of multiplication
-
starting the loop counter at 2 or 3 (if
integer is even or odd respectively)- fewer increments means fewer division operations (division is slow)
This integer add method replaces multiplication as such:
0*0 = 0
1*1 = 1 = 0 + 1
2*2 = 4 = 1 + 3
3*3 = 9 = 4 + 5
4*4 = 16 = 9 + 7
// etc.Revised function with explanations:
Factors::FactorsList Factors::calculate() const
{
FactorsList factors;
std::uint64_t incr = 0; // will increment by 2 each iteration
std::uint64_t incrSum = 1; // will accumulate value of 'incr' each iteration
// if integer is even, i starts at 2
// if integer is odd, i starts at 3
std::uint64_t i = (integer % 2 == 0) ? 2 : 3;
// 'i' is only incremented by 1 for checking each number
// value of 'incrSum' will determine loop termination
for (; incrSum <= integer; ++i)
{
if (integer % i == 0)
{
factors[i] = integer / i;
}
incr += 2;
incrSum += incr;
}
return factors;
}Code Snippets
0*0 = 0
1*1 = 1 = 0 + 1
2*2 = 4 = 1 + 3
3*3 = 9 = 4 + 5
4*4 = 16 = 9 + 7
// etc.Factors::FactorsList Factors::calculate() const
{
FactorsList factors;
std::uint64_t incr = 0; // will increment by 2 each iteration
std::uint64_t incrSum = 1; // will accumulate value of 'incr' each iteration
// if integer is even, i starts at 2
// if integer is odd, i starts at 3
std::uint64_t i = (integer % 2 == 0) ? 2 : 3;
// 'i' is only incremented by 1 for checking each number
// value of 'incrSum' will determine loop termination
for (; incrSum <= integer; ++i)
{
if (integer % i == 0)
{
factors[i] = integer / i;
}
incr += 2;
incrSum += incr;
}
return factors;
}Context
StackExchange Code Review Q#29932, answer score: 5
Revisions (0)
No revisions yet.