Зачем хранить знаменатель и другие выводимые поля в памяти, если знаменатель можно сделать всегда равным степенью двойки и хранить в виде параметра шаблона? Какой вообще смысл имеют знаменатели отличные от степеней двойки?
Здравствуйте, cppguard, Вы писали:
C>Зачем хранить знаменатель и другие выводимые поля в памяти, если знаменатель можно сделать всегда равным степенью двойки и хранить в виде параметра шаблона? Какой вообще смысл имеют знаменатели отличные от степеней двойки?
Тип называется Decimal, поэтому и знаменатель десятичный. Максимально десятичная семантика.
denum в принципе можно было бы и не хранить, а вычислять по месту из precision, это из разряда преждевременной оптимизации, просто по опыту решил, что это будет лучше. Ну и все равно, раз num — 64 бита, а для precision много не надо, то denum можно и кешировать в 32ух битной переменной.
Тут, имхо, можно было обойтись без DecimalDenumerator, и обойтись только DecimalPrecision и Decimal, ну тут хз, я предпочитаю прослойки делать почаще.
В качестве параметра шаблона точность хранить не вариант — точность у меня определяется в рантайме, и у меня можно в арифметических выражениях оперировать величинами разной точности, в результате возвращается результат с точностью, максимальной из той, что у есть операндов.
Я float/double конверсию специально сделал через строку — переложил ответственность за точность представления на стандартную либу std — "пусть лошадь думает, у неё голова большая". Потом можно будет и продумать получше этот нюанс, но пока float/doudle просто из кода пропали совсем.
Выходы за диапазоны (при всяких арифметических операциях в том числе) тоже не проверял — для финансовых расчётов всё равно никому верить нельзя, всё самому надо проверять, и входные данные, и выходное число. Тут наверное надо какие-то limits/traits допилить, чтобы было с чем сравнивать
Здравствуйте, Marty, Вы писали:
M>Здравствуйте!
M>Неохота было что-то искать, накидал на скорую руку M>...
Главное, что я не могу понять, это какой смысл было было создавать отдельные классы DecimalPrecision и DecimalDenumerator? ИМХО это переусложнение, это же просто целые числа, какой смысл делать для них классы обертки.
Зачем хранить и precision и denominator? denominator можно получить из precision очень простой операцией. Памяти твое число занимает в 1.5 раза больше, а по производительности выигрыш весьма сомнителен.
Как по мне лучше бы было без этих лишних оберток:
class Decimal {
//...int num;
int precision;
int denominator() const;
};
Ну, вместо int может нужны другие целые типы, но принцип понятен.
При делении, мы увеличиваем точность. Что ИМХО довольно быстро приведет к runtime_error если делить что-то в цикле.
А вот при умножении, точность не меняется, что будет если 0.5 * 0.5 ?
ИМХО правильнее перегружать операторы умножения и деления на целое число. А для умножения и деления двух Decimal нужно писать отдельные функции с разной стратегией изменения точности.
И еще Denumerator было бы неплохо переименовать в Denominator,
Ну и еще на первый взгляд вижу много всяких мелочей.
Здравствуйте, ksandro, Вы писали:
K>Главное, что я не могу понять, это какой смысл было было создавать отдельные классы DecimalPrecision и DecimalDenumerator? ИМХО это переусложнение, это же просто целые числа, какой смысл делать для них классы обертки.
DecimalPrecision — чтобы задавать точность в конструкторе и не перепутать с конструктором из целого.
DecimalDenumerator — да, возможно лишний
K>Зачем хранить и precision и denominator? denominator можно получить из precision очень простой операцией. Памяти твое число занимает в 1.5 раза больше, а по производительности выигрыш весьма сомнителен.
Ну, основное число у меня — int64_t. Я решил, что выравнивание на 8 лучше чем на 4 байта для int64_t. Впрочем, довольно необоснованное предположение, но есть вероятность, что так лучше, особенно на 64х-разрядных платформах. Так что можно считать это padding'ом до нужного размера.
K>При делении, мы увеличиваем точность. Что ИМХО довольно быстро приведет к runtime_error если делить что-то в цикле.
При делении у нас precision вычитаются, при умножении — складываются. После этих операций я привожу precision результата к большей из двух операндов. При сложении/вычитании — предварительно привожу к большей из двух, и так и оставляю.
K>А вот при умножении, точность не меняется, что будет если 0.5 * 0.5 ?
0.5 * 0.5 = 0.25, точность сложилась из двух. Я тут привожу к большей из двух, но вообще тут надо подумать, может стоит оставить суммарную. Вообще, этот decimal для денег сделал, там деньги на деньги редко умножаются. Обычно на целое количество. Хотя, если брать килограммы, то они не целые, да. Но у меня килограммов нет.
K>ИМХО правильнее перегружать операторы умножения и деления на целое число. А для умножения и деления двух Decimal нужно писать отдельные функции с разной стратегией изменения точности.
Так так вроде и сделано, не?
K>И еще Denumerator было бы неплохо переименовать в Denominator,
K>Ну и еще на первый взгляд вижу много всяких мелочей.
Code review не мой конек, к тому же вкусы у всех разные. Мне просто приходилось писать для денег несколько собственных похожих велосипедов, поэтому я смотрю на твою реализацию со своей колокольни, и может не понимаю некоторых деталей.
M>DecimalPrecision — чтобы задавать точность в конструкторе и не перепутать с конструктором из целого.
не понимаю, в чем проблема, у тебя второй precision второй параметр, он по умолчанию 0...
Что тут можно перепутать?
M>Ну, основное число у меня — int64_t. Я решил, что выравнивание на 8 лучше чем на 4 байта для int64_t. Впрочем, довольно необоснованное предположение, но есть вероятность, что так лучше, особенно на 64х-разрядных платформах. Так что можно считать это padding'ом до нужного размера.
Ну... ок, пусть будет так, хотя все равно мне это решение кажется несколько сомнительным, я бы так не делал.
K>>При делении, мы увеличиваем точность. Что ИМХО довольно быстро приведет к runtime_error если делить что-то в цикле. M>При делении у нас precision вычитаются, при умножении — складываются. После этих операций я привожу precision результата к большей из двух операндов.
Я не разбирался детально в коде. Но у тебя в комментах написано:
// 1200.00000 / 22.000 = 54.5454545454545
// 1200 00000 / 22 000 = 5454.54545454
// 123.010 / 13.10 = 9.390076335877862595419847328
Поэтому мне показалось, что ты делаешь подругому.
Не совсем понятно, почему они должны вычитаться... Но стратегию понял, ты всегда оставляешь бОльшую точность из двух чисел (делимого и делителя или множителя)
M>При сложении/вычитании — предварительно привожу к большей из двух, и так и оставляю.
по сложению вычитанию вопросов нет
K>>А вот при умножении, точность не меняется, что будет если 0.5 * 0.5 ?
M>0.5 * 0.5 = 0.25, точность сложилась из двух. Я тут привожу к большей из двух, но вообще тут надо подумать, может стоит оставить суммарную. Вообще, этот decimal для денег сделал, там деньги на деньги редко умножаются. Обычно на целое количество. Хотя, если брать килограммы, то они не целые, да. Но у меня килограммов нет.
Вот смотри, у тебя 0.5 * 0.5 будет 0.2 совершенно не факт что это именно то, что мы хотим.
K>>ИМХО правильнее перегружать операторы умножения и деления на целое число. А для умножения и деления двух Decimal нужно писать отдельные функции с разной стратегией изменения точности.
M>Так так вроде и сделано, не?
Не совсем, ты пишешь, что деньги умножать на деньги тебе не нужно. Поэтому я предлагаю операции "Decimal * Decimal" и "Decimal / Decimal" запретить, так как неочевидно, какая должна быть точность у результата.
Вместо них можно сделать оперишции "Decimal * int" и "Decimal / int". Этих операция достаточно для рассчетов типа "скидка 5%", "налог 13%" и тд. В результате сохраняется точность исходного Decimal (точность была в копейках, то и со скидкой будет в копейках). Но, если нам все-таки надо поделить или умножить Decimal между собой, то для этого можно сделать специальные функции типа:
Decimal mul(const Decimal& x, const Decimal& y, int precision);
Decimal div(const Decimal& x, const Decimal& y, int precision);
Это толжны быть именно функции или методы с явным указанием точности с которой мы хотим получить результат. Если мы хотим получить стоимость 2.5 кило товара, то мы скорее всего заранее знаем, что мы хотим получить ее с точностью до копейки (ну или до рубля), и нам неважно с какой точностью измеряется вес (до граммов или миллиграммов). Неявное приведение точности ведет к скрытым, труднообнаружимым ошибкам.
K>>Ну и еще на первый взгляд вижу много всяких мелочей.
M>Ну, можно и поподробнее
Разглядывать код внимательно мне неохота, так что напишу первое что заметил.
Я так понимаю, что Decimal(15, DecimalPrecision(3)) даст мне в результате 15.000. Как мне получить из целых чисел дробное? Это крайне актуальная задача при парсинге бинарных биржевых протоколов (типа ITCH). Там мы получаем цену в виде целого числа (например 12345) с известной точностью точностью (нпример 4 знака после запятой). Это значит что цена 1.2345.
Конструктор из числа с плавающей точкой лучше убрать. Ты конечно сделал конвертацию через строку, что позволяет избежать ошибок типа когда Decimal(0.3) превращается в 0.299999. Но все равно кто-нибудь обязательно сделает Decimal из результата вычислений с плавающей точкой типа Decimal(1.0/3.0) какая тут будет точность? А если так Decimal(1.0 / 30000000.0), тут вообще все будет очень весело!!!
Функуии toString и fromString можно бы сделать статическими методами, да и вообще можно бы сделать конструктор из строки (очень актуально для парсинга текстовых биржевых протоколов, типа FIX, ну или json).
В большинстве случаев точность можно задавать compile time через параметр шаблона, работать будет быстрее и памяти меньше занимать. Функцуиональности часто достаточно. Задал что точность 2 знака после запятой (копейки) и с такой точностью и считаешь. Но я не знаю, твою задачу, кое где может понадобиться и переменная точность.
Здравствуйте, ksandro, Вы писали:
K>Code review не мой конек, к тому же вкусы у всех разные. Мне просто приходилось писать для денег несколько собственных похожих велосипедов, поэтому я смотрю на твою реализацию со своей колокольни, и может не понимаю некоторых деталей.
Все равно спс
M>>DecimalPrecision — чтобы задавать точность в конструкторе и не перепутать с конструктором из целого. K>не понимаю, в чем проблема, у тебя второй precision второй параметр, он по умолчанию 0... K>
Очередность параметров. Где precision, а где value. Налюбится тут гораздо легче, чем кажется
M>>Ну, основное число у меня — int64_t. Я решил, что выравнивание на 8 лучше чем на 4 байта для int64_t. Впрочем, довольно необоснованное предположение, но есть вероятность, что так лучше, особенно на 64х-разрядных платформах. Так что можно считать это padding'ом до нужного размера. K>Ну... ок, пусть будет так, хотя все равно мне это решение кажется несколько сомнительным, я бы так не делал.
Ну и была бы кривая структура 96 бит размером. Тоже такое себе. Ну и какая-никакая, но просадка в производительности, когда каждый раз denum из precision делаешь. Но кривое выравнивание — это имхо бОльшая проблема
K>>>При делении, мы увеличиваем точность. Что ИМХО довольно быстро приведет к runtime_error если делить что-то в цикле. M>>При делении у нас precision вычитаются, при умножении — складываются. После этих операций я привожу precision результата к большей из двух операндов. K>Я не разбирался детально в коде. Но у тебя в комментах написано: K> // 1200.00000 / 22.000 = 54.5454545454545 K> // 1200 00000 / 22 000 = 5454.54545454 K> // 123.010 / 13.10 = 9.390076335877862595419847328 K>Поэтому мне показалось, что ты делаешь подругому. K>Не совсем понятно, почему они должны вычитаться... Но стратегию понял, ты всегда оставляешь бОльшую точность из двух чисел (делимого и делителя или множителя)
Да мне тоже думать было лень, было смутное подозрение, я его проверил, оно оказалось верным (на самом деле просто старые знания проверил).
Еще раз глянул код — перед делением я ещё делал minimizePrecision для делителя — когда лишние нули в конце после точки убираются.
Я в свое время делал трехбайтный float для спектрума, оттуда остаточные обрывки знаний и остались. Про IEE я тогда не знал, а возможно их и не было ещё, сам до всего доходил, двух байт показалось недостаточно для нужной точности, а на выравнивание там было пох.
M>>При сложении/вычитании — предварительно привожу к большей из двух, и так и оставляю. K>по сложению вычитанию вопросов нет
K>>>А вот при умножении, точность не меняется, что будет если 0.5 * 0.5 ?
M>>0.5 * 0.5 = 0.25, точность сложилась из двух. Я тут привожу к большей из двух, но вообще тут надо подумать, может стоит оставить суммарную. Вообще, этот decimal для денег сделал, там деньги на деньги редко умножаются. Обычно на целое количество. Хотя, если брать килограммы, то они не целые, да. Но у меня килограммов нет.
K>Вот смотри, у тебя 0.5 * 0.5 будет 0.2 совершенно не факт что это именно то, что мы хотим.
Тут я наврал сгоряча. При умножении изначально ничего не трогал, но сейчас ещё добавил minimizePrecision для делителя и для результата. Надо будет обновить на страничке сорцов.
K>>>ИМХО правильнее перегружать операторы умножения и деления на целое число. А для умножения и деления двух Decimal нужно писать отдельные функции с разной стратегией изменения точности.
M>>Так так вроде и сделано, не? K>Не совсем, ты пишешь, что деньги умножать на деньги тебе не нужно. Поэтому я предлагаю операции "Decimal * Decimal" и "Decimal / Decimal" запретить, так как неочевидно, какая должна быть точность у результата. K>Вместо них можно сделать оперишции "Decimal * int" и "Decimal / int". Этих операция достаточно для рассчетов типа "скидка 5%", "налог 13%" и тд. В результате сохраняется точность исходного Decimal (точность была в копейках, то и со скидкой будет в копейках). Но, если нам все-таки надо поделить или умножить Decimal между собой, то для этого можно сделать специальные функции типа: K>
K>Decimal mul(const Decimal& x, const Decimal& y, int precision);
K>Decimal div(const Decimal& x, const Decimal& y, int precision);
K>
K>Это толжны быть именно функции или методы с явным указанием точности с которой мы хотим получить результат. Если мы хотим получить стоимость 2.5 кило товара, то мы скорее всего заранее знаем, что мы хотим получить ее с точностью до копейки (ну или до рубля), и нам неважно с какой точностью измеряется вес (до граммов или миллиграммов). Неявное приведение точности ведет к скрытым, труднообнаружимым ошибкам.
Я тебя понял. "Decimal * Decimal" и "Decimal / Decimal" — точность авто, сейчас ещё добавил минимизацию точности при умножении. Идея в том, чтобы быть как встроенный тип без каких-либо нюансов, но хранить ровно то, то требуется. Тут нужно только указание точности при форматировании правильно указывать.
K>>>Ну и еще на первый взгляд вижу много всяких мелочей.
M>>Ну, можно и поподробнее
K>Разглядывать код внимательно мне неохота, так что напишу первое что заметил.
K>Я так понимаю, что Decimal(15, DecimalPrecision(3)) даст мне в результате 15.000. Как мне получить из целых чисел дробное? Это крайне актуальная задача при парсинге бинарных биржевых протоколов (типа ITCH). Там мы получаем цену в виде целого числа (например 12345) с известной точностью точностью (нпример 4 знака после запятой). Это значит что цена 1.2345.
Вот не понял, в чём проблема. Я как раз это замутил, потому что пишу плюсовый REST клиент под тинковское брокерское REST API
K>Конструктор из числа с плавающей точкой лучше убрать. Ты конечно сделал конвертацию через строку, что позволяет избежать ошибок типа когда Decimal(0.3) превращается в 0.299999. Но все равно кто-нибудь обязательно сделает Decimal из результата вычислений с плавающей точкой типа Decimal(1.0/3.0) какая тут будет точность? А если так Decimal(1.0 / 30000000.0), тут вообще все будет очень весело!!!
Тут я через жопу стандартной библиотеки сделал, да, и всю ответственность переложил на неё. Но Decimal from float/double не считаю нужным запрещать. Тут каждый сам себе буратино. Не считаю нужным отказывать желающим походить по граблям походить по ним.
K>Функуии toString и fromString можно бы сделать статическими методами, да и вообще можно бы сделать конструктор из строки (очень актуально для парсинга текстовых биржевых протоколов, типа FIX, ну или json).
Возможно, может пересмотрю как-нибудь. Пока я весь тинковский REST из какого-то сваггера смог в Qt/C++ оттранслировать, а все даблы из сгенерённого ручками заменил на свой Decimal. Это показалось мне более простым, чем править какой-то мутный генератор из сваггера, написанный на джаве, там всякие мавины сразу лезут, и вообще там полная пипецома
K>В большинстве случаев точность можно задавать compile time через параметр шаблона, работать будет быстрее и памяти меньше занимать. Функцуиональности часто достаточно. Задал что точность 2 знака после запятой (копейки) и с такой точностью и считаешь. Но я не знаю, твою задачу, кое где может понадобиться и переменная точность.
Я дергаю это через REST и сохраняю в SQL, так что ваще пофик на скорость. Переменная точность нужна просто для того, чтобы на разумных тащем-то значениях входных данных вылететь за пределы