bogdanov-d-a / oop_lab2

OOP lab 2
0 stars 0 forks source link

Замечания по генератору простых чисел #4

Open alexey-malov opened 9 years ago

alexey-malov commented 9 years ago
void PrimeNumbers::FillBitVector(BitVector &bitVector, bool fillBit, size_t count)
{
    assert(bitVector.empty());
    bitVector.reserve(count);
    for (size_t i = 0; i < count; ++i)
    {
        bitVector.push_back(fillBit);
    }
}

В классе std::vector один из конструкторов позволяет создать вектор нужного размера, заполненный заданным значением. И занимает это одну строчку http://www.cplusplus.com/reference/vector/vector/vector/

alexey-malov commented 9 years ago
void PrimeNumbers::FillBitVector(BitVector &bitVector, bool fillBit, size_t count)
{}

size_t PrimeNumbers::Sqr(size_t a)
{}

void PrimeNumbers::GenerateWithEratosthenesSieve(size_t upperBound, BitVector &bitVector)
{}

PrimeNumbers::NumberSet PrimeNumbers::GeneratePrimeNumbersSet(Number upperBound)
{}

Чтобы каждый раз не писать PrimeNumbers::blablabla можно сделать просто:

namespace PrimeNumbers
{
...
NumberSet GeneratePrimeNumbersSet(Number upperBound)
{
 ...
}
...
}
alexey-malov commented 9 years ago

PrimeNumbersUnit.h:

#pragma once

namespace PrimeNumbers
{
    typedef std::vector<bool> BitVector;
    typedef int Number;
    typedef std::set<Number> NumberSet;

    void FillBitVector(BitVector &bitVector, bool fillBit, size_t count);
    size_t Sqr(size_t a);
    void GenerateWithEratosthenesSieve(size_t upperBound, BitVector &bitVector);
    NumberSet GeneratePrimeNumbersSet(Number upperBound);
}

Вовсе не обязательно выносить в заголовочный файл все функции, в том числе, внутренние. Нужно выносить только те, которые требуются. В данном случае только метод, генерирующий множество.

Как следствие, кучи лишних typedef-ов можно было бы избежать. На мой взгляд, этого хедера вполне было бы достаточно:

#pragma once

namespace PrimeNumberGenerator
{
typedef std::set<int> IntSet;
IntSet GeneratePrimeNumbers(int upperBound);
}

Впрочем, даже namespace в такой простой программе, кажется лишним

alexey-malov commented 9 years ago

Купается мужик. Вдруг хватает его кто-то за яйца и голос спрашивает:

  • Плюс два или минус два? Мужик говорит:
  • Плюс два. Вылезает на берег, а у него четыре яйца. Думает мужик: сейчас вернусь, скажу "Минус два" и все опять будет хорошо. Полез он обратно в воду, доплыл до того места, там его опять хватают за яйца и спрашивают:
  • Плюс четыре или минус четыре?

В битовом массиве ради экономии двух битов вычитать двойку, а потом прибавлять при вставке во множество - источник ошибок при сопровождении данного кода сторонним программистом. 2 бита экономии памяти не стоят геморроя, связанного с сопровождением данного кода

alexey-malov commented 9 years ago
void GenerateWithEratosthenesSieve(size_t upperBound, BitVector &bitVector)

Чем не угодил возврат решета по значению?

BitVector GenerateWithEratosthenesSieve(size_t upperBound)
{
    BitVector sieve(upperBound + 1);
    // генерируем
    ...
    // компилятор догадается использовать перемещающий конструктор, работающий за O(1)
    return sieve; 
}
alexey-malov commented 9 years ago

Вместо BitVector лучше назвать тип более адекватно решаемой задаче: Sieve

alexey-malov commented 9 years ago

k=0,75 за исполнение 1,0 за срок