Hello!
On Friday 21 August 2009 00:11:56 Eugene V. Lyubimkin wrote:
> Alexey Pechnikov wrote:
> > Hello!
> >
> > On Thursday 20 August 2009 23:01:54 Eugene V. Lyubimkin wrote:
> >> Это когда некоторый математик садится за компьютер и начинает
> >> программировать
> >> алгоритм. А потом смотришь на код и понимаешь, что в проект (условно) его
> >> не
> >> взять. Он чудовищен.
> >
> > Хм, не задумывался, как мы пишем на С, но вроде откровенно ужасный код
> > писали всего
> > несколько студентов у нас... Теперь давайте по существу - а вы тесты
> > пишете? ;-)
> Да, случается.
>
> > Если вас не затруднит, приведите примеры своих разработок на С (его все
> > знают, как
> > эталон пойдет) - интересно увидеть и сравнить.
> nlkt - C++/Qt
> daptup - bash
> cupt - Perl
>
> На чистом С, извините, ничего нет. То, что его все знают - я бы не сказал,
> кстати.
>
> Всё в Debian, исходники, думаю, знаете как добыть.
Добыл первый из названных. Функционал выделен в класс, это хорошо.
Использование std имхо оверхед, легко можно избежать. В Logic.cpp
лучше бы обойтись без qt-х "прибамбасов" (QString и проч.), благо это
несложно, тогда код будет переносим и легче тестироваться/поддерживаться.
Вот этот кусок:
<-->plotLayout->addLayout(checkBoxesLayout, 0, 0, 1, -1);
<-->plotLayout->addWidget(this->exerciseCountPlot, 1, 0);
<-->plotLayout->addWidget(this->symbolCountPlot, 2, 0);
<-->plotLayout->addWidget(this->timePlot, 1, 1);
<-->plotLayout->addWidget(this->speedAveragePlot, 2, 1);
<-->plotLayout->addWidget(this->mistakeAveragePlot, 1, 2);
<-->plotLayout->addWidget(this->rhythmAveragePlot, 2, 2);
<-->plotLayout->addWidget(this->indexPlot, 3, 0, 1, -1);
явно кандидат в статический массив с циклом по нему и комментарием. Впрочем, это
мелочь.
А вот это очень мило:
============================
<-->std::vector<float> timeDistances;
<-->for (size_t i = 1; i < typingTimestamps.size()-1; ++i)
<-->{
<--><-->timeDistances.push_back(yf::get_milli_time(typingTimestamps[i],
typingTimestamps[i+1]));
<-->}
<-->float averageTime = std::accumulate(timeDistances.begin(),
timeDistances.end(), 0.0) / timeDistances.size();
<-->std::vector<float> timeDivergences;
<-->for (size_t i = 0; i < timeDistances.size(); ++i)
<-->{
<--><-->timeDivergences.push_back(fabs(averageTime - timeDistances[i]));
<-->}
<-->float divergence = 0.0;
<-->for (size_t i = 0; i < timeDivergences.size(); ++i)
<-->{
<--><-->divergence += timeDivergences[i] * timeDivergences[i];
<-->}
<-->divergence = sqrt(divergence) / timeDivergences.size();
============================
В то же время этот код по сути делает вот что:
============================
<-->float averageTime = 0.0;
<-->for (size_t i = 0; i < typingTimestamps.size()-2; i++)
<-->{
// для оптимизации можно создать массив для этих элементов
<--><-->averageTime += yf::get_milli_time(typingTimestamps[i],
typingTimestamps[i+1]);
<-->}
<-->averageTime = averageTime / (typingTimestamps.size()-1);
<-->float divergence_i, divergence = 0.0;
<-->for (size_t i = 0; i < typingTimestamps.size()-2; i++)
<-->{
// поскольку в квадрат потом возводим, модуль не нужно вычислять
<--><-->divergence_i = averageTime - yf::get_milli_time(typingTimestamps[i],
typingTimestamps[i+1])
<--><-->divergence += divergence_i * divergences_i;
<-->}
<-->divergence = sqrt(divergence) / (typingTimestamps.size()-1);
============================
Своих я за такие выкрутасы пинаю - незачем воротить тучу лишнего кода. Зато
все как у "взрослых" - std::vector, только вот не нужен он тут. std::fabs тоже
не нужен,
бессмысленно вычислять абсолютное значение для чисел, которые далее возводим в
четную степень. Использовать в каждом цикле новое условие тоже не нужно. ++i во
втором часу ночи не очень воспринимается, i++ было бы удобнее. Сдается мне,
что дивергенцию можно считать более оптимально, но это здесь не актуально.
Равно как и стоило бы сделать класс для хранения отсчетов, который при
добавлении
нового отсчета пересчитывает среднее и дивергенцию и предоставляет методы для их
получения - а иначе зачем вообще С++ использовать?
Это был первый взгляд... Наверняка если влезть глубже, то можно половину всего
кода выкинуть, равно как зависимость "движка" от qt и std, как в примере выше.
Но в текущем состоянии я бы такой код поддерживать не взялся.
> >
> > Из меня программист не ахти какой, моя работа больше архитектура ПО и
> > алгоритмика,
> > тем не менее, приведу ссылочку на свой код, например
> > http://mobigroup.ru/files/sqlite-ext/inet/
> Посмотрел по-диагонали. По мне - вполне себе неплохой код, читается нормально,
> вот только иногда странные цифры в названиях функций и код некоторых процедур
> выглядит подозрительно похожим (возможно, что-то общее можно выделить в общую
> функцию).
Да, разумеется. Но это делается во второй проход, когда код уже написан.
> Коды, про которые я говорил, были гораздо хуже. Ну, грубо говоря, типа:
>
> ((a*10.5*sqrt(x)-y*10.2)-(k+m-2087-m*(a-234.5)*a*pow(d,3)/a-c*b)/p*(e-505*k*a
> +y+0.4*sv(z))))-((u-234)*w*(z-sv(m*k)+ss((a+c*5)-q*6.7))*2
>
> И так "мелким почерком" на полторы страницы. Как такое поддерживать?
Так это вовсе не программа, это - мусор. Тут даже кодера нога не ступала...
Best regards, Alexey Pechnikov.
http://pechnikov.tel/
