7 сентября 2007 г.

Пример плохого кода

Сегодня гоняли тесты компонентов и обнаружили в логах "memory allocation error". Ну что ж, надо локализовывать... Анализ логов показал, что проблема возникает в MyClass::onInit. Решили начать с осмотра кода. Код, конечно, упрощен, но основной смысл остался:

bool MyClass::onInit(State state)
{
  if(!someObject.isReady())
  {
    std::cerr << "Can't register SomeObject."
      << std::endl;
    return false;
  }
  return BaseClass::onInit(state);
}

Похоже, что дальше нужно лезть в BaseClass::onInit(), потому как isReady() проверяет состояние объекта... Стоп! А что значит "Can't register SomeObject"? Причем тут регистрация? А ну-ка, что там в isReady()?..

...И обнаружили вот такой шедевр:

bool SomeObject::isReady()
{
  if(tryToRecover)
  {
    deinit();
    state = init(origin);
  }
  else if(!state)
  {
    state = init(origin);
  }
  return state;
}

Ошибка возникала при вызове init() — функции инициализации объекта. Но дело-то уже не в ошибке.

У меня нет слов... Засунуть в функцию проверки состояния объекта вызовы инициализации и деинициализации — это что-то. Я, конечно, понимаю, что это не конец света, но именно из таких мелочей складывается общая некрасивая картина.

Удивительно, как много книг и статей написано на эту тему. Удивительно, что в обсуждениях таких вопросов абсолютно все соглашаются, что так делать нельзя. И еще более удивительно, что в реальной работе огромное количество опытных(!) разработчиков продолжают так наплевательски относиться к своему собственному коду. Печально все это...

Комментариев нет:

Отправить комментарий