понедельник, 2 ноября 2009 г.

[comp.prog.flame] Вот так и сравнивают Erlang и C++

Наткнулся на интересную историю “Rewriting Playdar: C++ to Erlang, massive savings”, в которой автор Playdar рассказывает, как он переписал свой проект с C++ на Erlang и получил сокращение кода в 4-ре раза: 2K строк на Erlang вместо 8K строк на C++. Более того, версию на C++ писали около 6-ти месяцев, а переписали на Erlang за 2 недели. Внушаить, не правда ли?

У меня сразу закрались сомнения: при всей своей ущербности, C++ вовсе не такой язык, чтобы написанное на C++ за полгода можно было запросто переписать на каком-то другом языке за две недели. Такие вещи возможны только, если на C++ писали ламеры и если при этом они вручную реализовывали то, что для другого языка представлено в виде готовых инструментов.

Тезис о ламерстве в этой истории легко находит в данной истории свое подтверждение. Для этого достаточно заглянуть в старые C++ исходники: http://github.com/RJ/playdar

Их качество, как говорят, “ниже плинтуса” – у меня бы такой C++ код просто не прошел бы процедуру code review. Тогда как автор кода о своем творении более высокого мнения:

Изначально я написал Playdar на C++ (используя библиотеки Boost и Asio) начав в феврале этого года. Мне повезло работать с несколькими опытными разработчиками, которые помогли мне достичь результатов с C++. Среди нас было трое, кто регулярно писал на C++ еще несколько месяцев назад, и не смотря на то, что я относительный новичок в C++, я могу сказать, что мы получили хорошо спроектированный и надежный код, в котором все было предусмотрено.

I initially wrote Playdar in C++ (using Boost and Asio libraries), starting back in February this year. I was fortunate to be working with some experienced developers who helped me come to terms with C++. There were three of us hacking on it regularly up until a few months ago, and despite being relatively new to C++, I’ll say that we ended up with a well designed and robust codebase, all things considered.

Не откажу себе в удовольствии заглянуть в некоторые места этого “well designed and robust” кода. Пойдем сразу в файл main.cpp. Мы там видим функцию main() на 110(!) строк, в которой выполняются сразу несколько действий, которые более грамотный разработчик обязан был вынести в разные функции: обработка аргументов командной строки, чтение конфигурации, инициализация библиотеки curl, создание класса MyApplication (классное название, не правда ли?), запуск встроенного в приложение HTTP-сервера.

И ладно бы, каждое из этих действий было бы сделано нормально, так ведь нет. Встречаются замечательные перлы:

bool error;
try {
    po::parsed_options parsedopts_cmd =
       po::command_line_parser(ac, av).options(cmdline_options).run();
    store(parsedopts_cmd, vm);
    error = false;
} catch (po::error& ex) {
    // probably an unknown option.
    cerr << ex.what() << "\n";
    error = true;
}

Назначить переменной error значение true или false сразу нельзя было ну никак.

Или вот еще:

app = new MyApplication(conf);
// start http server:
string ip = "0.0.0.0"; 
boost::thread http_thread(
   ... );
        
http_thread.join();
log::info() << "HTTP server finished, destructing app..." << endl;
delete app;
log::info() << "App deleted." << endl;
return 0;

Зачем было создавать экземпляр MyApplication динамически? Он же единственный на всю программу, создается и удаляется в одном и том же месте. Проще всего было бы объявить его прямо на стеке, как обычную локальную переменную. Но даже если захотелось создать его динамически, какая религия запретила обернуть его в std::auto_ptr?

Взглянем на класс MyApplication. Будь автор этого класса моим подчиненным, он уже бы отгреб за одно название. Что не позволило назвать его PlaydarApplication – понять невозможно. Разве что остается вспомнить, что выбор имен классов/методов/переменных – это одна из самых сложных задач в программировании. Ну да ладно с названием, смотрим на сам класс. Его объявление:

class MyApplication
{
public:
    MyApplication(Config c);
    ~MyApplication();
 
    Resolver * resolver();
    
    Config * conf()
    {
        return &m_config;
    }
    
    // RANDOM UTILITY FUNCTIONS TOSSED IN HERE FOR NOW:
    
    // swap the http://DOMAIN:PORT bit at the start of a URI
    std::string http_swap_base(const std::string& orig, const std::string& newbase)
    {
        if(orig.at(0) == '/') return newbase + orig;
        size_t slash = orig.find_first_of('/', 8); // https:// is at least 8 in
        if(std::string::npos == slash) return orig; // WTF?
        return newbase + orig.substr(slash);
    }
    
    // functor that terminates http server:
    void set_http_stopper(boost::function f) { m_stop_http=f; }
    
    void shutdown(int sig = -1);
    
private:
    
    boost::function m_stop_http;
    Config m_config;
    Resolver * m_resolver;
 
};

Сразу возникают вопросы:

  • почему аргумент передается в конструктор по значению? Почему нельзя было передавать его по константной ссылке?
  • почему методы resolver() и conf() возвращают указатели? Простые указатели, а не константные указатели или ссылки. Почему сами методы не константные?
  • зачем в классе MyApplication неконстантный метод http_swap_base? Он не обращается к внутренностям объекта вообще. Он должен был быть внешней свободной функцией или, в крайнем случае, статическим методом класса (возможно, приватным);
  • странно, что значение для m_stop_http не передается в конструкторе класса. Вряд ли работа класса возможна, если предварительно не вызвать set_http_stopper;
  • не видно методов-сеттеров, которые устанавливали бы значение атрибута m_resolver. Это наводит на мысль о том, что данный атрибут динамически создается в конструкторе.

Смотрим на реализацию класса:

MyApplication::MyApplication(Config c)
    : m_config(c)
{
    string db_path = conf()->get("db", "");
    m_resolver  = new Resolver(this);
}
 
MyApplication::~MyApplication()
{
    log::info() << "DTOR MyApplication" << endl;
    log::info() << "Stopping resolver..." << endl;
    delete(m_resolver);
}
     
void
MyApplication::shutdown(int sig)
{
 
    cout << "Stopping http(" << sig << ")..." << endl;
    if(m_stop_http) m_stop_http();
}
 
Resolver *
MyApplication::resolver()
{
    return m_resolver;
}

Классная строчка в конструкторе с локальной переменной db_path. Дернули конфигурацию, получили значение и все. Похерили это значение нафиг. Молодцы.

А m_resolver действительно создается динамически. И вручную удаляется в деструкторе. Здорово. Про boost::function знаем, про std::auto_ptr не знаем.

Еще не понятно, по каким принципам код методов располагается в .h и .cpp файлах. Почему MyApplication::conf() реализован прямо в .h-файле, а точно такой же MyApplication::resolver() – в .cpp-файле?

Отдельного разговора заслуживает обработка исключений. В показанной выше функции main() исключения ловятся по ссылке. Странно, что не по константной ссылке, но хоть по ссылке. А вот в файле auth.cpp исключение sqlite3pp::database_error ловится уже по значению! А уж бросаются исключения вообще козырно:

        if( val != "2" )
        {
            throw; // not caught here
        }

Я, например, и не знал, что инструкцию throw можно использовать вот так. Т.е. повторно пробросить уже пойманное исключение с помощью простого throw – это нормально. Но чтобы с его помощью выбрасывать новое исключение… Век живи, век учись. А в дополнение к вышесказанному большое количество catch(…){} в коде.

Остается добавить для полноты картины факты возврата объектов типа vector<map<string, string>> или передачи аргументов типа map<string, string> по значению и получится довольно полная картина того, что автор называет well designed and robust codebase.


С качеством кода, можно сказать, закончили. Теперь о том, зачем вообще потребовалось делать Playdar на C++?

Как я понимаю, Playdar – это такая маленькая музыкальная поисковая система. Висит на машине в виде http-сервера, к ней обращаются с запросом, она производит поиск и выдает результат в виде JSON-ответа. Зачем здесь C++?

Первый вариант: скорость работы Playdar. Вряд ли C++ может ее ускорить – это не вычислительная задача и не обработка больших объемов данных.

Второй вариант: Playdar должен потреблять минимум ресурсов. Ну да, возвращая вектора мапов по значению можно сильно выиграть в расходе ресурсов :-/

Третий вариант: минимализация дистрибутива. Дистрибутив C++ного варианта был 2.5Mb. Дистрибутив Erlang-ового – 10Mb. Имхо, разница серьезная.

Итак, выбор C++ мог быть здесь оправданным только, если автор хотел получить минимальное потребление ресурсов (чему помешало незнание С++) и/или малый размер дистрибутива (но, судя по приемлимости 10Mb-го дистрибутива, это его не интересовало).

Из чего напрашивается вывод, что для данной конкретной задачи C++ вообще не подходил изначально (добавим сюда и верные слова автора о геморрое, связанном с бинарными плагинами). Ее можно было гораздо успешнее решить на каком-нибудь Python или Ruby. И разрыв между этими языками и Erlang-ом был бы не так велик.

Итого, что в сухом остатке? Человек, который не умеет программировать на C++ почему-то взялся делать свой проект на C++, хотя этот язык для данной задачи не подходил изначально. И еще хорошо, что он вовремя отказался от развития C++ной версии в пользу Erlang-а (хотя Erlang-овую версию я не смотрел, не исключено, что и там качество кода…). Но зато какой громкий заголовок получился!


Почему меня эта история зацепила так, что я решился на очередной “крестовый поход в защиту C++”? А тем, что хочется видеть противопоставление нормального C++ кода нормальному коду на других языках. Но никак не такие ламерские поделки.

PS. По моим впечатлениям, плохой код, который активно использует boost::function, гораздо тяжелее читать, чем просто плохой код.

PPS. Имхо, если бы я переписал этот Playdar на C++ нормально (возможно с использованием Poco вместо Boost), то объем кода получился бы раза в полтора больше. За счет разбиения многих функций на более мелкие и за счет объектной декомпозиции вместо функциональной. Но на написание такой версии ушло бы намного меньше шести месяцев – может быть, как раз те самые две недели.

3 комментария:

Alexander P. комментирует...

Ну там видно, что не весь код писали такие ламеры. То есть в других файлах, всё-таки видно, что люди знают что-то о передачи параметра по const-ссылке =).

Хотя читать это не просто, и кодового единобразия (Например: где BOOST_FOREACH, а где простой цикл с итераторами).

А code review, видно, да не используют. Как, наверное и в 70% open-source на коленке.

eao197 комментирует...

Там, имхо, код писало два класса начинающих C++ программистов:

1. Ламеры, которые не знают про std::auto_ptr, константные ссылки и пр.

2. Ламеры, которые уже об этом знают, но не знают, что C++ными наворотами нельзя злоупотреблять.

Unknown комментирует...

Отличный разбор полётов. Так их! -)