HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Remake of factoring/prime factorization calculator

Submitted by: @import:stackexchange-codereview··
0
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:

  • 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 a namespace.



  • Can both calculate()s be better optimized, considering they perform numerous divisions?



  • I'm not quite liking the call to calculate() in both display()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, and calculate() should still be private.



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;
};

#endif


Factors.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;
};

#endif


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

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.