The daily Axapta WTF

[Stitch_MS]

Цитата:

Сообщение от kashperuk

А вот как наши доблестные девелоперы пофиксили это

Выглядит так, будто доблестные девелоперы умеют писать скрипты, но не любят проверять результаты их работы.

[kashperuk]

Цитата:

Сообщение от Stitch_MS

Выглядит так, будто доблестные девелоперы умеют писать скрипты, но не любят проверять результаты их работы.

Если бы дело было только в этом.
Это изменение отревьюило 2 человека.. То есть втроем, с учетом того что сам девелопер тоже правил, они не смогли нормально скобочки сосчитать, и их никак не удивила эта странная констркция со сравнением двух энумов..

[skuull]

Цитата:

Сообщение от kashperuk

Если бы дело было только в этом.
Это изменение отревьюило 2 человека.. То есть втроем, с учетом того что сам девелопер тоже правил, они не смогли нормально скобочки сосчитать, и их никак не удивила эта странная констркция со сравнением двух энумов..

А сколько ещё изменений было в этом ревью ? Там же есть зависимость между длинной изменения и качеством его ревью

[kashperuk]

Цитата:

Сообщение от skuull

А сколько ещё изменений было в этом ревью ? Там же есть зависимость между длинной изменения и качеством его ревью

Еще где-то 10 других изменений. Я их побоялся смотреть

[ax_mct]

Цитата:

Сообщение от kashperuk

Если бы дело было только в этом.
Это изменение отревьюило 2 человека.. То есть втроем, с учетом того что сам девелопер тоже правил, они не смогли нормально скобочки сосчитать, и их никак не удивила эта странная констркция со сравнением двух энумов..

Не в скобках дело, а в полном абзаце мозга. Никакие тесты не помогут если мы проверяем потенциально на ноль одно значение, а делим на другое. При втором вызове функция может возвратить другое значение. C вероятностью практически нулевой конечно, но мы же программисты или где

AmountMST cost = this.itemPrice(ModuleInventPurchSales::Invent);
    AmountMST sales = this.itemPrice(ModuleInventPurchSales::Sales);
    ...

    //Avoid division by zero
    if (sales == 0)
    {
        mcrCatalogAreaAnalysisTmp.CogsPercent = 0;
    }
    else
    {
        mcrCatalogAreaAnalysisTmp.CogsPercent = cost / sales;
    }

[macklakov]

Цитата:

Сообщение от ax_mct

Не в скобках дело, а в полном абзаце мозга. Никакие тесты не помогут если мы проверяем потенциально на ноль одно значение, а делим на другое.

Да не, он пытался проверить тоже самое значение которое попадет в знаменатель. Но зачем-то решил его вычислить повторно и при этом перепутал местоположение скобки. Код должен был выглядеть вот так:
if (this.itemPrice(ModuleInventPurchSales::Sales) == 0)
Такое случается по запаре. Ничего особенного.
Но вот фикс шедеврален! Это же человек не просто колотил код на скорость. Это вдумчиво выловили баг, подумали над его природой и исправили. Кажется я начинаю понимать почему местный саппорт так неохотно репортит даже доказанные баги с указанием где что и как поломано в продуктовую команду.

[Ace of Database]

Цитата:

Сообщение от macklakov

Но вот фикс шедеврален! Это же человек не просто колотил код на скорость. Это вдумчиво выловили баг, подумали над его природой и исправили.

Как раз не вдумчиво. Механически заменили константу 0 на его аналог в енуме. Это чистой воды копи-пастинговый, рутинный подход. Программисты любят копи-паст, как ни крути.

[VORP]

Да нет, не копи паст это. Просто человек пишет компилятор, фреймворк для екстеншенов для енумов. Он не знает и/или скорее всего не хочет знать ничего про цены и прочие MCR - этим кодом владеет команда MCR(условно говоря). Сейчас он поменял что-то в компиляторе и у него в некоторых местах ошибки компилятора. Править чужой код он не может, но надо сделать чтоб компилилось. Вот он и заменил цифру на енум, хоть оно и бестолково. По хорошему должен был кинуть багу на MCR что код походу неправильный. Но опять же, не факт что команда MCR будет править код, ведь клиенты баги не репортят, а там мало ли что кому кажется.

[ax_mct]

Цитата:

Сообщение от VORP

Да нет, не копи паст это. Просто человек пишет компилятор, фреймворк для екстеншенов для енумов. Он не знает и/или скорее всего не хочет знать ничего про цены и прочие MCR - этим кодом владеет команда MCR(условно говоря). Сейчас он поменял что-то в компиляторе и у него в некоторых местах ошибки компилятора. Править чужой код он не может, но надо сделать чтоб компилилось. Вот он и заменил цифру на енум, хоть оно и бестолково. По хорошему должен был кинуть багу на MCR что код походу неправильный. Но опять же, не факт что команда MCR будет править код, ведь клиенты баги не репортят, а там мало ли что кому кажется.

Скорее всего так и есть. Была ошибка компилятора - её и пофиксили.
Но мне знаете ли страшно жить с такими системными программистами.
Там ведь не вопрос бизнес-логики, а деление на ноль. Поверяем одно значение, делим на другое.
Проверяем на ноль второй результат вызова функции, а делим с первым результатом вызова этой же функции. Для нормального программиста С/С++ мимо такого пройти просто невозможно. Это просто режет глаз.

[dech]

Цитата:

Сообщение от VORP

Да нет, не копи паст это. Просто человек пишет компилятор, фреймворк для екстеншенов для енумов. Он не знает и/или скорее всего не хочет знать ничего про цены и прочие MCR - этим кодом владеет команда MCR(условно говоря). Сейчас он поменял что-то в компиляторе и у него в некоторых местах ошибки компилятора. Править чужой код он не может, но надо сделать чтоб компилилось. Вот он и заменил цифру на енум, хоть оно и бестолково. По хорошему должен был кинуть багу на MCR что код походу неправильный. Но опять же, не факт что команда MCR будет править код, ведь клиенты баги не репортят, а там мало ли что кому кажется.

Да... Чем крупнее компания, тем сложнее поддерживать стабильный код. Здесь ведь и до этого компилилось, это же рантайм еррор. Думаю, где-то просто тесты не сработали. И потом видимо выдали на доработку уже не тому, кто этим кодом занимался.
В большой компании должно быть разграничение кода между специалистами, чтобы отвечать за свой участок и знать его как свои 4 пальца. :-)

[macklakov]

Цитата:

Сообщение от VORP

Править чужой код он не может, но надо сделать чтоб компилилось. Вот он и заменил цифру на енум, хоть оно и бестолково. По хорошему должен был кинуть багу на MCR что код походу неправильный. Но опять же, не факт что команда MCR будет править код, ведь клиенты баги не репортят, а там мало ли что кому кажется.

В принципе, выглядит очень правдоподобно. Только похоже что писали они не компилятор как таковой, а best practice check. Ведь единственный результат изменения в том, что теперь в коде нет константы "0". Можно даже предположить что это робот сделал.