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

[alexey-malov]

1>c:\teaching\ips\2018\oop\glazyrin\oop\lab3\car\car\car.cpp(55): warning C4715: 'CCar::TurnOffEngine': not all control paths return a value
1>c:\teaching\ips\2018\oop\glazyrin\oop\lab3\car\car\car.cpp(122): warning C4715: 'CCar::SetSpeed': not all control paths return a value

• [x] Программа должна компилироваться без предупреждений компилятора во всех конфигурациях и платформах.

[alexey-malov]

> Info
  Engine     :   Off
  Gear       :   0
  Speed      :   0
  Direction  :   Stop
> EngineOn
> Info
  Engine     :   On
  Gear       :   0
  Speed      :   0
  Direction  :   Stop
> SetGear -1
> SetSpeed 10
> Info
  Engine     :   On
  Gear       :   -1
  Speed      :   10
  Direction  :   Back  <---------- Тут еще едем назад
> SetGear 0
> Info
  Engine     :   On
  Gear       :   0
  Speed      :   10
  Direction  :   Forward  <---------- Направление резко сменилось на противоположное
>

• [x] Метод не покрыт тестами, в него прокралась ошибка

[alexey-malov]

• [x] Нет теста, проверяющего невозможность изменить направление движения без полой остановки

[alexey-malov]

• [x] В CDriveCar::HandleCommand() можно обойтись одним action map-ом, если функции, исполняющие команды будут принимать поток, из которого будут самостоятельно считывать нужные им аргументы

[alexey-malov]

bool CDriveCar::SetSpeed(int speed)
{
    int gear = CCar::GetGear();
    int currentSpeed = CCar::GetSpeed();
    if (!IsEngineTurnedOn())
    {
        std::cout << "  Error: Engine should be turned on" << std::endl;
    } else if ((gear == 0) && (speed > currentSpeed) && (speed >= 0)) {
        std::cout << "  Error: Cannot set speed more than current if gear equals 0" << std::endl;
    } else if (speed < 0) {
        std::cout << "  Error: Speed cannot be negative" << std::endl;
    } else if (!IsCorrectValue(gear, speed)) {
        std::cout << " Error: Speed is not correct for this gear" << std::endl;
    }

    CCar::SetSpeed(speed);
    return true;
}

• [x] Программа не соответствует принятому стилю кодирования. Ознакомьтесь с правилами именования переменных, функций, классов, расстановкой скобок, знаков препинания и отступов.

[alexey-malov]

class CDriveCar : public CCar
{

• [x] Использование наследования здесь неуместно

[alexey-malov]

• [ ] Покрыть тестами DriveCar

[alexey-malov]

• [ ] Покрыть тестами красную ветку

[alexey-malov]

• [ ] покрыть тестами

[alexey-malov]

• [ ] покрыть тестами красную ветку

[alexey-malov]

• [ ] Программа должна выводить сообщения об ошибках, если действие выполнить не удалось. Сейчас не (всегда) выводит. Например, при попытке включить передачу, недопустимую в текущих условиях (например, при езде задом включить переднюю передачу)

[alexey-malov]

• [ ] При вводе конца файла (Ctrl+Z) программа не должна выводить "Unknown Command"

[alexey-malov]

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

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());
    ...
}

[alexey-malov]

bool CDriveCar::SetGear(std::istream & args)
{
    int gear;
    string input;
    args >> input;

    int speed = m_car.GetSpeed();
    int currentGear = m_car.GetGear();

    if (IsConvertStringToInteger(input, gear))

• [ ] Здесь и в других местах следует считывать сразу в переменные нужных типов (int)

[alexey-malov]

• [ ] При проверке неизвестной команды надо ожидать, что HandleCommand вернёт False, а автомобиль останется в том же состоянии, что и до её вызова.