Cinderella-Man / hands-on-elixir-and-otp-cryptocurrency-trading-bot

Source code to generate the "Hands-on Elixir & OTP: Cryptocurrency trading bot" book
https://elixircryptobot.com
263 stars 34 forks source link

Typo at the end of 2.3.2 (i think) #7

Closed gabrielgrover closed 3 years ago

gabrielgrover commented 3 years ago

Reading through your book. Really enjoying it so far. I came across this piece of code at the end of 2.3.2

defp calculate_sell_price(buy_price, profit_interval, tick_size) do
    fee = D.new("1.001")
    original_price = D.mult(D.new(buy_price), fee)

    net_target_price =
      D.mult(
        original_price,
        D.add("1.0", profit_interval)
      )

    gross_target_price = D.mult(net_target_price, fee)

    D.to_float(
      D.mult(
        D.div_int(gross_target_price, tick_size),
        tick_size
      )
    )
  end

However the commentary that is provided immediate after is a little confusing. For instance there is this quote,

We started by calculating the gross_buy_price which is a sum of buy price together with the fee that we paid on top of it.

I do not see gross_buy_price, but perhaps that was intended to be original_price, however the original_price variable is not calculate with a sum like the above quote states it is D.mult(D.new(buy_price), fee). There is another quote that is similar,

As we will be charged a fee for selling, we need to add the fee again on top of this target sell price(gross_target_price).

Again, that is referring to the line gross_target_price = D.mult(net_target_price, fee) which is also a multiplication and not a sum.

Cinderella-Man commented 3 years ago

Hello :wave:

Thank you very much for opening the issue. I can see your point and I'm thinking about how could I improve this section to make it easier to understand. The obvious upgrade would be to match the variable names :wink:

We started by calculating the original_price which is a sum of the buy price together with the fee that we paid on top of it.

however the original_price variable is not calculate with a sum like the above quote states it is D.mult(D.new(buy_price), fee)

The calculation is correct and indirectly we are adding the fee to the initial buy price(as the fee is a ratio value at this moment). It could be coded as:

D.add(
  buy_price,
  D.mult(buy_price, fee)
)

Would this make it easier to understand? :thinking:

Cinderella-Man commented 3 years ago

My first take on this would be:

  defp calculate_sell_price(buy_price, profit_interval, tick_size) do
    fee = "0.001"

    original_price = D.mult(buy_price, fee)

    net_target_price =
      D.mult(
        original_price,
        D.add("1.0", profit_interval)
      )

    gross_target_price = D.mult(net_target_price, fee)

    D.to_float(
      D.mult(
        D.div_int(gross_target_price, tick_size),
        tick_size
      )
    )
  end

First, we will hardcode the fee to 0.1% which we will refactor in one of the future chapters.

We started by calculating the original_price which is the buy price together with the fee that we paid on top of it.

Next, we enlarge the originally paid price by profit interval to get net_target_price.

As we will be charged a fee for selling, we need to add the fee again on top of the net target sell price(we will call this amount the gross_target_price).

Next, we will use the tick size as Binance won't accept any prices that aren't divisible by the symbols' tick sizes so we need to "normalize" them on our side.

gabrielgrover commented 3 years ago

My first take on this would be:

  defp calculate_sell_price(buy_price, profit_interval, tick_size) do
    fee = "0.001"

    original_price = D.mult(buy_price, fee)

    net_target_price =
      D.mult(
        original_price,
        D.add("1.0", profit_interval)
      )

    gross_target_price = D.mult(net_target_price, fee)

    D.to_float(
      D.mult(
        D.div_int(gross_target_price, tick_size),
        tick_size
      )
    )
  end

First, we will hardcode the fee to 0.1% which we will refactor in one of the future chapters.

We started by calculating the original_price which is the buy price together with the fee that we paid on top of it.

Next, we enlarge the originally paid price by profit interval to get net_target_price.

As we will be charged a fee for selling, we need to add the fee again on top of the net target sell price(we will call this amount the gross_target_price).

Next, we will use the tick size as Binance won't accept any prices that aren't divisible by the symbols' tick sizes so we need to "normalize" them on our side.

I like this approach. Makes a lot more sense to me. Thanks again for such an awesome resource!

Cinderella-Man commented 3 years ago

Thank you very much for your kind words and for opening this issue. It will be merged to the main branch and released in the new version 0.1.5.

Thanks once again and please feel free to open issues whenever you would see any other places with room for improvement