Hi everybody,
I found a bug or structural problem in Decimal what could have some big impact:
wrong calculation (maybe with money!!!) or even infinite loop ;-(
My suggestion for a fix is on pull request:
Does anybody have another idea - there are some possible ways to handle validation for structs - but what would you suggest is the best way?
Any suggestions are welcome!
Here are one solution example for division:
defmodule Decimal do
@type coefficient :: non_neg_integer | :NaN | :inf
@type exponent :: integer
@type sign :: 1 | -1
@type t :: %__MODULE__{
sign: sign,
coef: coefficient,
exp: exponent
}
defstruct sign: 1, coef: 0, exp: 0
...
def div(%Decimal{coef: coef1}, %Decimal{})
when coef1 < 0,
do: error(:invalid_operation, "dividend (#{coef1}) must be > 0", %Decimal{coef: :NaN})
def div(%Decimal{}, %Decimal{coef: coef2})
when coef2 < 0,
do: error(:invalid_operation, "divisor (#{coef2}) must be > 0", %Decimal{coef: :NaN})
the orig pull request text - if donāt want to follow the link:
Hi,
I found a bug or structural problem in Decimal what could have some big impact under special circumstances. Here an example from the Money lib where Decimal is used as main backbone:
iex(15)> m = Money.new(:USD, Decimal.new(%Decimal{sign: -1, coef: -12000000000}))
#Money<:USD, --12000000000>
iex(16)> Money.mult(m, 3)
{:ok, #Money<:USD, --36000000000>}
iex(17)> Money.mult!(m, 3 )|> Money.to_string
{:ok, "$--*,000,000,000.00"}
Of course itās a special edge case ā¦ normally the best practice is:
iex(18)> m = Money.new(:USD, "-12000000000")
#Money<:USD, -12000000000>
but you never know ā¦ maybe someone using it ā¦itās possible! itās valid Code ! NO compiler warning! NO runtime error!
And the worst case, if you are using any division with this kind of Decimal number like this:
iex(19)> m = Money.new(:USD, Decimal.new(%Decimal{sign: -1, coef: -12000000000}))
#Money<:USD, --12000000000>
iex(20)> Money.mult!(m, 3) |> Money.to_decimal |> Decimal.to_float
-> you end up in an infinite loop!!!
The reason for this is clear: the %Decimal{} struct - with no validation for input -> so you can input a negative coefficient.
The type spec and docu are perfect - but do not prevent any misuse:
Type spec : @type coefficient :: non_neg_integer | :NaN | :inf
Documentation: The coefficient of the power of 10
. Non-negative because the sign is stored separately in sign
.
So my suggestion for a solution you can find in this pull request: mostly checks for this special edge case.
And this time I included the corresponding tests.
Kind regards and I hope it helps anybody to use Decimal in a safer way!
Ralph