8
\$\begingroup\$

I am a student in Germany, more specifically in Bavaria, and here we use to have a so-called "Kärwa", an event for celebrating the establishment of the local church. It is common to sell "Bratwürste" or sausages on these events. Since I am a student and I need the money, I often help with the sale.

Normal cashiers calculate the price and the return money in their head, but since I am neither good nor fast with Math, I decided to open a python interpreter I had installed on my mobile and made this program. The main aim is to be able to calculate the price fast and have all of the necessary information displayed.

For review: I come from a java/c#/c++ background and I know I can't really treat python the same way, but with the limited knowledge of python I had, I tried my best. If there are little features in the language I can use or there is a best practice, tell me about it. I would mainly like to hear about the clarity, documentation, and implementation of my code, especially for the calc_money_sizes function.

The code may be a bit "over-engineered" this is due to some long periods where I had nothing to do and got bored. But still don't hold back on criticism I can take it :)

__author__ = "Patrick Hollweck" 

import collections

class Money:
  """ A class handling Interaction with money """

  SIZES = [0.01, 0.020, 0.050, 0.10, 0.20, 0.5, 1.0, 2.0, 5.0, 10.0, 20.0, 50.0, 100.0, 200.0, 500.0]

  def calc_money_sizes(money):
    result = collections.OrderedDict()
    if money < 0:
      return result
    for size in reversed(Money.SIZES):
      while True:
        outcome = money - size
        if outcome > -0.001:
          if size in result:
            result[size] += 1
          else:
            result[size] = 1
          money = outcome
        else:
          break
    return result

class PriceCalculator:
  """ Class to calculate all sort of bratwurst Stuff! """

  SAUSAGE_PRICE = 1.10
  BREAD_PRICE = 0.30

  def calc_price(sausage_count, bread_count):
    sausage_price = sausage_count * PriceCalculator.SAUSAGE_PRICE
    bread_price = bread_count * PriceCalculator.BREAD_PRICE
    return sausage_price + bread_price

  def calc_return_money(price, given):
    return given - price


class Console:
  """ Console helpers """

  class input:
    """ Input helpers """

    def safe_int(message, allowNoInput = False):
      return Console.input.__safe_base(message, allowNoInput, int)

    def safe_float(message, allowNoInput = False):
      return Console.input.__safe_base(message, allowNoInput, float) 

    def __safe_base(message, allowNoInput, typefunc):
      while True:
        try:
          result = input(message)
          if result == "" and allowNoInput:
            return None
          elif result == "clear":
            Console.Out.clear()
            continue
          else:
            return typefunc(result)
        except Exception:
          print("Invalid input! (" + str(typefunc) + ") expected")
          continue

  class format:
    """ Formating functions for the console """

    def float(number):
      return "{0:.2f}".format(number)

    def str_normalize_lenght(string, longest):
      if len(string) < longest:
        missing_chars = longest - len(string)
        return string + " " * abs(missing_chars)
      else:
        return string

  class pretty_print:
    """ Use this class for pretty printing commons types """ 

    def float(number):
      print(Console.format.float(number))

    def dict(dictionary):
      if len(dictionary) is 0:
        print("NOT POSSIBLE")
        return

      for key in dictionary.keys():
        print(str(Console.format.str_normalize_lenght(Console.format.float(key), 6)) + " -> " + str(dictionary[key]))


  class output:
    """ Output helpers """

    def header(text):
      print("\n--- " + text + " ---\n")

    def clear():
      print("\n" * 50)


if __name__ == "__main__":

  Console.output.clear()

  while True:

    # print header
    print("Bratwurst price Calculator!\n")

    # Get input
    sausage_count = Console.input.safe_int("Sausages Count: ")
    bread_count = Console.input.safe_int("Bread Count: ")

    # Calculate total price and output
    price = PriceCalculator.calc_price(sausage_count, bread_count) 

    Console.output.header("TOTAL PRICE")
    Console.pretty_print.float(price)

    # Calculate return money 
    given = Console.input.safe_float("\nGiven money: ", allowNoInput = True)

    # Optional skip return money calculation when reqested
    if given == None:
      Console.output.clear()
      continue

    return_money = PriceCalculator.calc_return_money(price, given)

    Console.output.header("RETURN MONEY")

    # Return money amount
    if return_money < 0:
      print("WARNING: Not enought money!")

    Console.pretty_print.float(return_money)

    # Return money format
    Console.output.header("RETURN MONEY FORMAT")
    Console.pretty_print.dict(Money.calc_money_sizes(return_money))

    # Cleanup
    input("\n... Continue ...")
    Console.output.clear();
\$\endgroup\$
  • 3
    \$\begingroup\$ None of your classes are actually classes; they're just namespaces. \$\endgroup\$ – Zeta Feb 6 '18 at 22:39
  • 1
    \$\begingroup\$ Just a suggestion, you might want to obfuscate your email address there :) \$\endgroup\$ – hjpotter92 Feb 7 '18 at 0:12
7
\$\begingroup\$

You wanted something fast?

This doesn't look fast:

# Get input
sausage_count = Console.input.safe_int("Sausages Count: ")
bread_count = Console.input.safe_int("Bread Count: ")

What if the person only bought bread? Now you are forced to enter 0 for Sausages Count, and only then will you be able to enter the bread count.

That doesn't sound fast to me.

I would suggest you opt for a kind of parser-reader method. For example, you can say that S is for Sausage and B is for bread - all case insensitive of course. Then you can simply enter:

10S4b

OR

4b10s

This should translate to 10 sausages and 4 bread.

You can also enter 10s or 4b.

With this syntax, you are no longer forced to enter both items if the person did not buy both. It is also less typing because you only have to press ENTER once.

To make this work, you may want to use the regex library, and regex like this:

import re

regex = re.compile(r'(\d+[sS])|(\d+[bB])')

def get_input():
    s = input()
    sausage, bread = [], []
    for match in regex.finditer(s):
        if match.group(1) is not None:
            sausages.append(match.group(1)[:-1])
        if match.group(2) is not None:
            bread.append(match.group(2)[:-1])

    return sum(map(int, sausage)), sum(map(int, bread))

Granted I haven't tested this, but the expected result is that you will get back a tuple where the first value is the number of sausages and the second is the number of bread.

Test it, try it, improve it if you must.

I was initially going to suggest writing the letter first and then the number, but it occurred to me that your phone's auto-correct might kick in and try to correct the spelling of whatever word it thought you are trying to spell.


@staticmethod

Your classes seem to not have any state being stored in them, so you should make it known that they are simply static methods in your class.

You can do so by annotating the methods with the @staticmethod decorator


Something cannot be equal to nothing, because that will make nothing, something.

If you didn't parse that correctly, I am referring to this:

if given == None:
   ...

It is usually frowned upon to check if a variable has a value by comparing it to None. Some even consider it an anti-pattern.

The recommended way of doing it is by using the is operator, so:

if given is None:
    ...

It's all semantics, but if you don't get it, no worries, it's not the end of the world, I will just have to remind you again next time I see it :).


Builtins

This method you have here:

def str_normalize_lenght(string, longest):
  if len(string) < longest:
    missing_chars = longest - len(string)
    return string + " " * abs(missing_chars)
  else:
    return string

...is already built into the string class. It is called rjust. So replace all uses of your over-engineered function with str.rjust(longest), or change the function to use rjust.


Faster, faster everywhere...

This code for calculating change is not as fast as it could get

def calc_money_sizes(money):
    result = collections.OrderedDict()
    if money < 0:
      return result
    for size in reversed(Money.SIZES):
      while True:
        outcome = money - size
        if outcome > -0.001:
          if size in result:
            result[size] += 1
          else:
            result[size] = 1
          money = outcome
        else:
          break
    return result

You will not even notice the difference, but you did say you wanted this code to be fast, so here is my improvement to achieve that sweet spot of fast and readable:

def calc_money_sizes(money):
    result = collections.OrderedDict()
    for size in reversed(Money.SIZES):
      outcome = money / size
      if int(outcome) > 0:
        result[size] = int(outcome)
        money %= size
    return result

General comments:

  • Avoid naming your variables and functions with the same name as some builtin methods or modules. Example, float, string, input.

  • Your use of classes is interesting. You use them like one would use namespaces in C++ or C#, but I don't think most python users will agree with it. I don't mind it, but I think you may want to look into modules and see if that's the kind of structure you were going for.

\$\endgroup\$
  • 4
    \$\begingroup\$ I love the "Something cannot be equal to nothing, because that will make nothing, something" quote. 10/10 will reuse :) \$\endgroup\$ – hjpotter92 Feb 7 '18 at 10:08

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.