6
\$\begingroup\$

The input for price() would be STRINGS, as you see below. If the string starts with "-" I would like what follows to be stored in fiat_name and then retrieve the symbol from _KNOWN_FIAT_SYMBOLS. "--rub" (EXAMPLE) could be at any position in the list passed through price()

If the optional fiat_name does not exist in _KNOWN_FIAT_SYMBOLS i would like it to default to USD/$

I would like the code optimized and cleaned/minimized. I am trying to practice clean and read-able code.

import re

_KNOWN_FIAT_SYMBOLS = {"USD":"$", "RUB":"₽"} # this will be populated with more symbols/pairs later


def price(*arguments):
    # default to USD/$
    fiat_name = "USD"

    arguments = list(arguments)
    cryptos = []

    for arg in arguments:
        arg = arg.strip()
            
        if not arg:
            continue

        for part in arg.split(","):
            if part.startswith("-"):

                fiat_name = part.upper().lstrip("-")
                    
                continue
                
            crypto = re.sub("[^a-z0-9]", "", part.lower())
                
            if crypto not in cryptos:
                cryptos.append(crypto)

    if not cryptos:
        cryptos.append("btc")
            
    fiat_symbol = _KNOWN_FIAT_SYMBOLS.get(fiat_name)
        
    if not fiat_symbol:
        fiat_name = "USD"
        fiat_symbol = "$"

    print(f"{cryptos} to: {fiat_name}{fiat_symbol}")


price("usd", "usdc", "--rub") # ['usd', 'usdc'] to: RUB₽ (becuase of the optional --rub)
price("usd,usdc,eth", "btc", "-usd") # ['usd', 'usdc', 'eth', 'btc'] to: USD$ (becuase of the optional --usd)
price("usd", "usdc,btc", "-xxx") #['usd', 'usdc', 'btc'] to: USD$ (because xxx does not exist in _KNOWN_FIAT_SYMBOLS
price("usd,usdc,eth", "btc") # ['usd', 'usdc', 'eth', 'btc'] to: USD$ (becuase no optional fiat_name was given)
price("usd,--rub,eth", "btc") # ['usd', 'eth', 'btc'] to: RUB₽ (becuase of the optional --rub)
price("--rub") # ['btc'] to: RUB₽ (becuase of the optional --rub and the cryptos is empty)
price("") # ['btc'] to: USD$ (becuase of the default USD and the cryptos is empty)
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. Thanks! \$\endgroup\$ Jul 9 at 13:29
9
\$\begingroup\$

Return data. Algorithmic code should return data, not print – at least whenever feasible. It's better for testing, debugging, refactoring, and probably other reasons. At a minimum, return a plain tuple or dict. Even better would be a meaningful data object of some kind (e.g., namedtuple, dataclass, or attrs class). Print elsewhere, typically in a function operating at the outer edge of your program.

Function *args are already independent tuples. That means you don't need arguments = list(arguments).

Algorithmic readability. Your current code is generally fine: I had no great difficulty figuring out how it was supposed to work. However, the parsing rules themselves have a tendency to require an annoying amount of conditional logic: I was not happy with the first draft or two I sketched out. But I did come up with an alternative that I ended up liking better: one function focuses on parsing just the currency names (a fairly narrow, textual task); and the other part deals with the annoyances of missing/unknown data. Note that both functions return meaningful data objects – an important tool in writing more readable code – and the second function handles missing/unknown values with a "declarative" style. Overall, this version strikes me as somewhat more readable. If you need to optimize for raw speed, it's probably slower.

import re
from collections import namedtuple

_KNOWN_FIAT_SYMBOLS = {"USD":"$", "RUB":"₽"}

FiatCryptos = namedtuple('FiatCryptos', 'name symbol cryptos')
Currency = namedtuple('Currency', 'name is_fiat')

def price(*arguments):
    currencies = list(parse_currencies(arguments))
    cryptos = [c.name for c in currencies if not c.is_fiat] or ['btc']
    fiats = [c.name.upper() for c in currencies if c.is_fiat] or ['USD']
    f = fiats[0]
    syms = _KNOWN_FIAT_SYMBOLS
    name, sym = (f, syms[f]) if f in syms else ('USD', '$')
    return FiatCryptos(name, sym, cryptos)

def parse_currencies(arguments):
    for arg in arguments:
        for part in arg.strip().split(','):
            if part:
                is_fiat = part.startswith('-')
                name = re.sub('[^a-z0-9]', '', part.lower())
                yield Currency(name, is_fiat)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.