How Would You Make This Code Snippet Cleaner?

Questions / Help
1

I’m still trying to break my Python if/then habit. I have 3 transformations in this function. I’ve tried several solutions to get the logic I want in a clean way. They all seem kinda messy. I don’t know which direction to go next to get it cleaner. How would you clean this code?

def get_price_precision(price, fuzz_offset) do

    price_digits_before_decimal_with_value = 
        case price >= 1.0 do
            true -> get_digits_before_decimal(price)
            false -> 0
        end

    fuzz_offset_dig_past_decimal = get_digits_past_decimal(fuzz_offset)

    optimize_precision = price_digits_before_decimal_with_value - (fuzz_offset_dig_past_decimal + 1)
    optimize_precision = 
        case optimize_precision > 0 do
            true -> 0
            false -> abs(optimize_precision)
        end 

    price_dig_past_decimal = get_digits_past_decimal(price)

    case price_dig_past_decimal > optimize_precision do
        true -> price_dig_past_decimal
        false -> optimize_precision
    end
end
2

Please excuse me for not answering your question just yet but are you using float to represent a price?

Elixir has no built in support for decimal data type or decimal calculations (rounding, cutting off after nth digit etc) but Decimal library is widely used. For example, if you use Ecto, you can store price in Decimal field in database and it will get mapped to Decimal from this library.

Accidentally, using the library may simplify your code snippet :wink:

1 Like
3

I use floats for the bulk of the module. I had to use Decimals for some numbers formatting. But still unsure when to use floats, when to use Decimals. Advantage of floats over decimals is memory right? Any other disadvantage I’d need to consider using Decimals?

4

Ogod, never ever use floats ever ever ever with money ever in any case, ever in any language in any system ever, like Ever!

I hope I can sum up why with a single example:

iex> (1.1 + 2.2) === 3.3
false

You will get the same answer in every language (assuming you have precise comparisons enabled).

This is just a small small sampling of the huge iceberg that are floating point. It all stems from the fact that a lot of number are just not representable in binary floating point (see the wikipedia article). Your numbers slowly diverge the more you operate on them, which is fine for games (well, not really there either, I’m sure you’ve seen physics explosions in games, that is usually becomes of floating point inaccuracies, but games need speed, so they give up accuracy for speed), but NOT fine for money. O.o

Never ever ever use floating point numbers for anything where accuracy is important, like ever.

I hope this post is clear enough, there is never ever ever a case to ever use floating point where accuracy is important, like ever ever ever. Feel free to google for more information and examples of cases and cases that have near destroyed companies. :slight_smile:

5 Likes
5

You could use if instead of case when the results are booleans. Even better you can use pattern matching to get rid of it.

your code:

case price >= 1.0 do
  true -> get_digits_before_decimal(price)
  false -> 0
end

better:

if price >= 1.0 do 
  get_digits_before_decimal(price) 
else 
  0
end

even better

def get_price_precision(price, fuzz_offset) do
  price_digits_before_decimal_with_value = get_digits_before_decimal(price)
end

defp get_digits_before_decimal(price) when price >= 1.0, do: ...
defp get_digits_before_decimal(price), do: 0

this case:

case price_dig_past_decimal > optimize_precision do
  true -> price_dig_past_decimal
  false -> optimize_precision
end

could become:

Enum.max([price_dig_past_decimal, optimize_precision])

There are some other things…like using more pipes and assign less variables. But this would require some refactoring…

6

iex(2)> 0.01 + 0.06
0.06999999999999999

Whops. @Emily as a rule of thumb, when dealing with money do stick to either integers (and operate on cents) or use decimals with appropriate precision. Otherwise you can loose money :wink:

1 Like
7

@Emily on topic of this offtopic - for prices you can use this lib https://github.com/liuggio/money - it’s pretty cross-language thing to go extra mile to represent money :slight_smile: As for the mentioned library - we used it in production, it’s good

8

It looks like Decimal & Money are 3rd party. Which is more stable? If you had to choose 1 or the other, what would you use?

(Also, I’m doing calculations with currencies denominated in BTC, which can often be 8 or 9 decimal point precisions.)

9

Decimal is about as first class as a library can get. It has full support in Ecto and many of the primary devs, it will exist and be updated.

Money is however money-centric, it has a lot of stuff that will make sure things are ‘done right’, it is the traditional 3-rd party lib, but it has such a defined role that if the dev ever stopped updating it then someone else would take it up anyway.

Well you could define it to 25 decimal points or whatever if you want, or 100. :slight_smile:

The EVM/BEAM has arbitrary sized integers so representing such values in such libraries actually remains quite efficient.

EDIT: Also, for note, 32-bit floating point with 8-9 decimal places you are losing precision, you do not want floating point especially for that. With 64-bit it goes further, but still you are not able to represent numbers accurately so you still do not want floating point.

1 Like
10

To concur, we encountered that doing some 3rd party app for eve online. When you count distance in meter in space in js (for a webgl stuff) from the center if the sun of the system you are in… the smallest mistakes propagate fast and give strange things…

1 Like
11

Any guidelines to follow, deciding when to use case? When to use if/then? When to breakup the function & use a when pattern match?

12

Here’s, another refactored version., minimizing the case clauses, & breaking up the function…

(It might be my python roots, but the first version to me, I can read easier & make out the transformations faster then the second. But consensus seems to be something more like the code below would be cleaner?

def get_price_precision(price, fuzz_offset) do
    price_digits_before_decimal_with_value = get_price_digits_before_decimal(price)
    fuzz_offset_dig_past_decimal = get_digits_past_decimal(fuzz_offset)
    optimize_precision = calc_opt_price_precision(fuzz_offset_dig_past_decimal, price_digits_before_decimal_with_value)
    price_dig_past_decimal = get_digits_past_decimal(price)
    Enum.max([price_dig_past_decimal, optimize_precision])
end
defp get_price_digits_before_decimal(price) when price >= 1.0, do: get_digits_before_decimal(price)
defp get_price_digits_before_decimal(_), do: 0
defp calc_opt_price_precision(fuzz_offset_dig_past_decimal, price_digits_before_decimal_with_value) do
    optimize_precision = price_digits_before_decimal_with_value - (fuzz_offset_dig_past_decimal + 1)
    if optimize_precision > 0, do: 0, else: abs(optimize_precision) 
end

(It still needs refactoring to use Decimals.)

13

Basically:

  • Use if/else when you only do a true/false check.
  • Use case when you have a different check, or multiple clauses.
  • As soon as possible, and especially if you would otherwise start nesting if, unless or case, put the logic of it in its own function. The main advantage of this is that the logical steps you are performing now has a name.
14

What does get_price_precision/2 do?

15

Returns the price precision to compare against order book sample price precision. It’s used to optimize the final price precision & ending price digits to increase fill probability.

16

At the end of the day, “is the source code readable” is probably more important than “is this perfectly idiomatic”. So I wouldn’t go overboard with multi-headed functions just because the language supports it as an alternative way for decision making.

And indeed, if you don’t want an external library, convert everything you get in into integer µBTC (or some smaller size depending on the needed precision) and stay with integer math. Floats are for science and physics, never for money. It takes some getting used at (I once worked on a system where money was in m€ (tenths of a Eurocent), which was funny at first but really helpful in the long run).

2 Likes