ewildgoose / elixir-xml_rpc

Encode and decode elixir terms to XML-RPC parameters
Apache License 2.0
40 stars 17 forks source link

Features/add formatted float struct #16

Closed EevanW closed 4 years ago

EevanW commented 5 years ago

Issue: When we converting float to string, we can see repeating decimal there should be a fixed number of characters after point (128.39 -> "128.38999999999999"). For example:

iex(1)>  %XMLRPC.MethodCall{method_name: "test", params: [127.39]} |> XMLRPC.encode!  
"<?xml version="1.0" encoding="UTF-8"?>
<methodCall>
  <methodName>test</methodName>
  <params>
    <param>
      <value>
        <double>127.39</double>
      </value>
    </param>
  </params>
</methodCall>"
iex(2)>  %XMLRPC.MethodCall{method_name: "test", params: [128.39]} |> XMLRPC.encode!  
"<?xml version="1.0" encoding="UTF-8"?>
<methodCall>
  <methodName>test</methodName>
  <params>
    <param>
      <value>
        <double>128.3888888889</double>
      </value>
    </param>
  </params>
</methodCall>"

The problem lies in the declared accuracy of calculations :erlang.float_to_binary This problem can be solved by using [options] as parameter of encoding/2 function.

For example:

    tag("double", Float.to_string(double, options
                                          |> Keyword.put_new(:compact, true)
                                          |> Keyword.put_new(:decimals, 14))

But it only generic solution, because we may need many different float values.

My final solution - formatted struct: %XMLRPC.FormattedFloat.new({1.2345, "~.2f"}) |> XMLRPC.to_binary() -> "1.23"

ewildgoose commented 5 years ago

What happens if you use a Decimal type in here instead?

How does it work if we simply use:

tag("double", Float.to_string(double))

This seems to work ok for my limited testing here? I think the implementation of Float.to_string has improved since the original implementation used above. Now it has a better pretty printer which mostly seems to round correctly as far as my brief checks show?

ewildgoose commented 5 years ago

Please see if this latest commit resolves your concern? It also introduces the option to use Decimal types (which allow you to perfectly control rounding in the way you desire)

edit: https://github.com/ewildgoose/elixir-xml_rpc/commit/ec05f52e923b0f74d939045bc8625ab485f4898e

(Updated commit as I have no idea what I was doing before... Editing and running "mix test" on a totally different project... oops.)

EevanW commented 5 years ago

Nope. It's not work.

iex> 0..15000 |> Enum.map(&Float.to_string(&1/100+0.09))
["0.09", "0.09999999999999999", "0.11", "0.12", "0.13", "0.14", "0.15", "0.16",
 "0.16999999999999998", "0.18", "0.19", "0.2", "0.21", "0.22", "0.23", ...]

0..15000 |> Enum.map(&(&1/100+0.09) |> Decimal.from_float |> Decimal.reduce() |> Decimal.to_string())      
["0.09", "0.09999999999999999", "0.11", "0.12", "0.13", "0.14", "0.15", "0.16",
 "0.16999999999999998", "0.18", "0.19", "0.2", "0.21", "0.22", "0.23", ...]

iex(2)> 0..15000 |> Enum.map(&(&1/100+0.09) |> Decimal.cast |> Decimal.reduce() |> Decimal.to_string())            
["0.09", "0.09999999999999999", "0.11", "0.12", "0.13", "0.14", "0.15", "0.16",
 "0.16999999999999998", "0.18", "0.19", "0.2", "0.21", "0.22", "0.23", ...]

Unfortunately it's low level issue. You can see it if you tests: Float.to_string or :erlang.float_to_binary or inspect(). Pass global option for XMLRPC.ValueEncoder.encode(nil, option) nave a own problem if we want construct XML with different types of values. For example: as 55.1234567 as -7.1234567 and as 1.23

EevanW commented 5 years ago

Oops.. my test is incorrect. I sends divide result but it contain many decimals. I will test you last commit again.

EevanW commented 5 years ago

Ok. It is work correctly. Thanks! May be problem in case when need value ["<double>", "131.0000", "</double>"] because Float reduces trailing zeroes ["<double>", "131.0", "</double>"]. But it is not my case.