Замечания по программе Car

[alexey-malov]

• [x] Убрать из класса CCar ввод/вывод

[alexey-malov]

        /*set possible speed just for current gear*/
        REQUIRE(true == car.SetGear(1));
        REQUIRE(true == car.SetSpeed(19));
        REQUIRE(false == car.SetGear(1));
        REQUIRE(false == car.SetGear(2));

        /*set impossible speed for current gear*/
        REQUIRE(false == car.SetSpeed(31));

        /*set impossible gear for current speed*/
        REQUIRE(false == car.SetGear(2));

        /*set speed to increase gear*/
        REQUIRE(true == car.SetSpeed(20));
        REQUIRE(true == car.SetGear(2));

        /*set possible speed just for current gear*/
        REQUIRE(true == car.SetSpeed(31));

        /*reach max speed*/
        REQUIRE(true == car.SetSpeed(50));
        REQUIRE(true == car.SetGear(5));
        REQUIRE(true == car.SetSpeed(150));
        REQUIRE(false == car.SetSpeed(155));

        /*decrease gear*/
        REQUIRE(false == car.SetGear(4));
        REQUIRE(true == car.SetSpeed(90));
        REQUIRE(true == car.SetGear(4));

        /*set negative speed*/
        REQUIRE(false == car.SetSpeed(-100));

• [x] Тесты должны не только проверять успешность или неудачу при выполнении модифицирующей состояние объекта операции, но еще и сравнивать состояние объекта после операции с ожидаемым. Если в разных тестах используются однотипные проверки состояния (состоящие из нескольких действий), то полезным будет выделить их в функции вида:

void ExpectOperationSuccess(SomeObj & obj, const function<bool(SomeObj & obj)> & op, int expectedProperty1, const string & expectedProperty2, ...)
{
    BOOST_REQUIRE(op(obj)); // ожидаем, что операция вернет true (успех)
    // Сравниваем состояние свойства объект с ожидаемым
    BOOST_CHECK_EQUAL(obj.GetProperty1(), expectedProperty1);
    BOOST_CHECK_EQUAL(obj.GetProperty2(), expectedProperty2);
    ...
}

void ExpectOperationFailure(const SomeObj & obj, const function<bool(SomeObj & obj)> & op);
{
    auto clone(obj); // сделали клон объекта
    BOOST_REQUIRE(!op(clone)); // ожидаем, что операция завершится неуспешно (передаем клон)
    // проверяем, что после выполнения операции состояние клона не отличается от оригинала 
    // (операция в случае неудачи оставляет объект в состоянии, в котором он пребывал до операции)
    BOOST_CHECK_EQUAL(clone.GetProperty1(), obj.GetProperty1());
    BOOST_CHECK_EQUAL(clone.GetProperty2(), obj.GetProperty2());
    ...
}

• [x] Тесты должны проверять граничные условия (значения на самой границе и в ее окрестностях).

[alexey-malov]

• [x] Вместо PrintState добвить Get-методы для доступа к состоянию

[alexey-malov]

Info
Engine state:ON, Gear:0, Speed:10, Direction:FORWARD
SetSpeed 9
Info
Engine state:ON, Gear:0, Speed:9, Direction:BACK

• [x] Неожиданная смена направления движения.
• [x] Воспроизвести проблему в тестах (это можно сделать после вынесения Get-методов).

[alexey-malov]

bool Car::GearCorrespondsToSpeed(const int gear) const
{
    GearSpeed speedInterval = transmission.find(gear)->second;
    return speedInterval.minSpeed == speedInterval.maxSpeed || Between(m_speed, speedInterval.minSpeed, speedInterval.maxSpeed);
}

• [x] Если не хотите проверять итератор, то отдайте предпочтение методу at. Он бросит исключение, если ключ не будет найден

[alexey-malov]

• [x] При движении задом должно быть запрещено включать переднюю (любую) передачу (что на нейтрали, что на задней передаче)

• [x] Для этой ситуации должны быть написаны тесты

[alexey-malov]

• [x] Посмотрите, как организован диалог с пользователем в примере с классом CTVSet

[alexey-malov]

• [x] Таблица диапазонов скоростей на передачах должна быть одна на все автомобили

[alexey-malov]

• [x] Из класса автомобиль убрать преобразование enum-ов в строку. Этим должен заниматься класс CarControl

    std::string EngineStateToString(const EngineState state) const;
    std::string DirectionToString(const Direction direction) const;

• [ ] Сделать эти методы приватными статическими методами класса, либо объявить в безымянном пространствен имён внутри .cpp файла контроллера автомобиля

[alexey-malov]

enum EngineState
{
    ON,
    OFF
};

enum Direction
{
    FORWARD,
    BACK,
    NONE
};

• [x] Использовать scoped enum

[alexey-malov]

Engine state:ON, Gear:0, Speed:0, Direction:NONE
EngineOff
Failed to turn on engine. Engine is already active

• [x] Невозможно выключить двигатель.
• [ ] Покрыть тестами эту ситуацию

[alexey-malov]

• [ ] Имена тестов должны звучать как спецификация на тестируемый класс, а не просто перечислять тестовые ситуации:

Сценарий: Смена направления движения возможна только при полной остановке
 Дано: автомобиль движущийся задом на нейтральной скорости
 Когда: пытаемся включить переднюю передачу
 Тогда: автомобиль сообщает об ошибке

[alexey-malov]

• [ ] По возможности размеры тестов должны быть небольшими.

[alexey-malov]

• [ ] Добавить тесты для контроллера автомобиля (посмотреть как сделано в примере с телевизором)