Skip to content

Address all Leandro's feedback #39

@anxolin

Description

@anxolin

I'll accumulate in this issue, all the good tips from Leandro.
Since there's MANY waterfall PRs and approved with nits and comments, I'll make this issue so I merge all and then address the suggested comments :)

1. Use var for price:

  • Done?

Nit: I would put the prices into their own variables and simplify the string building to something like:

price = format_price(
          calculate_price(
            numerator=boughtVolume,
            denominator=soldVolume,
            decimals_numerator=buyTokenDecimals,
            decimals_denominator=sellTokenDecimals
          ), 
          currency=buyTokenLabel
        )
priceStr1 = f'{click.style(f'  Avg. Traded Price {sellTokenLabel}/{buyTokenLabel}', fg=labelColor)}: {price1}'

...

tradePriceText = f'{priceStr1}\n{priceStr2}\n'

#32 (comment)

2. Max sell volume, max receiving amount

  • Done?

Would it be too much to also add the order size? Meaning, max buy/sell amounts, not just the matched amounts.

#32 (comment)

3. Use f strings for formatting numbers

  • Done?

def format_integer(number):
return str(number) # TODO: Format better the numbers
return '{:,d}'.format(number)

Also in:

  return f'{{:,.{decimals}f}}'.format(rounded_value).rstrip('0').rstrip('.')

Also in:
return format_amount(percentage, decimals=2) + '%'
#34 (comment)

4. Unreachable code in calculate_price

  • Done?

#32 (comment)

5 Simplify expression

6. Improve sorting

7. Use snake case for vars

  • Done?

Not comment from leandro, but I saw it now :)
isUnlimitedAmount

8. Improve for comprehension

Python magic 🌟

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions