2
\$\begingroup\$

I'm making a calculator that is supposed to calculate the price of fruits in a trade (for a game)

omg, my code is so bad and messy that its embarrassing. I'm so sorry for anyone who has to look at this

def add(u, x, y, z):
   return u + x + y + z

fruit1 = input("Enter first fruit: " )
fruit2 = input("Enter second fruit: ")
fruit3 = input("Enter third fruit: ")
fruit4 = input("Enter fourth fruit: ")

# fruit 1
if fruit1 == "blank":
   fruit1 = 0




elif fruit1 == "kilo":
   fruit1 = 7500


elif fruit1 == "spin":
   fruit1 = 10000


elif fruit1 == "chop":
   fruit1 = 100000


elif fruit1 == "spring":
   fruit1 = 20000


elif fruit1 == "bomb":
   fruit1 = 30000


elif fruit1 == "smoke":
   fruit1 = 200000


elif fruit1 == "spike":
   fruit1 = 100000


elif fruit1 == "flame":
   fruit1 = 450000


elif fruit1 == "falcon":
   fruit1 = 200000


elif fruit1 == "ice":
   fruit1 = 800000


elif fruit1 == "sand":
   fruit1 = 750000


elif fruit1 == "dark":
   fruit1 = 1000000


elif fruit1 == "revive":
   fruit1 = 450000


elif fruit1 == "diamond":
   fruit1 = 550000


elif fruit1 == "light":
   fruit1 = 900000


elif fruit1 == "love":
   fruit1 = 350000


elif fruit1 == "rubber":
   fruit1 = 400000


elif fruit1 == "barrier":
   fruit1 = 400000


elif fruit1 == "magma":
   fruit1 = 1100000


elif fruit1 == "quake":
   fruit1 = 1100000


elif fruit1 == "buddha":
   fruit1 = 2300000


elif fruit1 == "string":
   fruit1 = 1600000


elif fruit1 == "phoenix":
   fruit1 = 1700000


elif fruit1 == "portal":
   fruit1 = 2200000


elif fruit1 == "rumble":
   fruit1 = 2450000


elif fruit1 == "paw":
   fruit1 = 1750000


elif fruit1 == "blizzard":
   fruit1 = 2700000


elif fruit1 == "gravity":
   fruit1 = 2600000


elif fruit1 == "dough":
   fruit1 = 4400000


elif fruit1 == "shadow":
   fruit1 = 3300000


elif fruit1 == "venom":
   fruit1 = 4000000


elif fruit1 == "control":
   fruit1 = 2600000


elif fruit1 == "spirit":
   fruit1 = 2500000


elif fruit1 == "dragon":
   fruit1 = 4200000


elif fruit1 == "leopard":
   fruit1 = 7800000


# fruit 2
if fruit2 == "blank":
   fruit2 = 0




elif fruit2 == "kilo":
   fruit2 = 7500




elif fruit2 == "spin":
   fruit2 = 10000


elif fruit2 == "chop":
   fruit2 = 100000


elif fruit2 == "spring":
   fruit2 = 20000


elif fruit2 == "bomb":
   fruit2 = 30000


elif fruit2 == "smoke":
   fruit2 = 200000


elif fruit2 == "spike":
   fruit2 = 100000


elif fruit2 == "flame":
   fruit2 = 450000


elif fruit2 == "falcon":
   fruit2 = 200000


elif fruit2 == "ice":
   fruit2 = 800000


elif fruit2 == "sand":
   fruit2 = 750000


elif fruit2 == "dark":
   fruit2 = 1000000


elif fruit2 == "revive":
   fruit2 = 450000


elif fruit2 == "diamond":
   fruit2 = 550000


elif fruit2 == "light":
   fruit2 = 900000


elif fruit2 == "love":
   fruit2 = 350000


elif fruit2 == "rubber":
   fruit2 = 400000


elif fruit2 == "barrier":
   fruit2 = 400000


elif fruit2 == "magma":
   fruit2 = 1100000


elif fruit2 == "quake":
   fruit2 = 1100000


elif fruit2 == "buddha":
   fruit2 = 2300000


elif fruit2 == "string":
   fruit2 = 1600000


elif fruit2 == "phoenix":
   fruit2 = 1700000


elif fruit2 == "portal":
   fruit2 = 2200000


elif fruit2 == "rumble":
   fruit2 = 2450000


elif fruit2 == "paw":
   fruit2 = 1750000


elif fruit2 == "blizzard":
   fruit2 = 2700000


elif fruit2 == "gravity":
   fruit2 = 2600000


elif fruit2 == "dough":
   fruit2 = 4400000


elif fruit2 == "shadow":
   fruit2 = 3300000


elif fruit2 == "venom":
   fruit2 = 4000000


elif fruit2 == "control":
   fruit2 = 2600000


elif fruit2 == "spirit":
   fruit2 = 2500000


elif fruit2 == "dragon":
   fruit2 = 4200000


elif fruit2 == "leopard":
   fruit2 = 7800000
   
# fruit 3
if fruit3 == "blank":
   fruit3 = 0




elif fruit3 == "kilo":
   fruit3 = 7500




elif fruit3 == "spin":
   fruit3 = 10000


elif fruit3 == "chop":
   fruit3 = 100000


elif fruit3 == "spring":
   fruit3 = 20000


elif fruit3 == "bomb":
   fruit3 = 30000


elif fruit3 == "smoke":
   fruit3 = 200000


elif fruit3 == "spike":
   fruit3 = 100000


elif fruit3 == "flame":
   fruit3 = 450000


elif fruit3 == "falcon":
   fruit3 = 200000


elif fruit3 == "ice":
   fruit3 = 800000


elif fruit3 == "sand":
   fruit3 = 750000


elif fruit3 == "dark":
   fruit3 = 1000000


elif fruit3 == "revive":
   fruit3 = 450000


elif fruit3 == "diamond":
   fruit3 = 550000


elif fruit3 == "light":
   fruit3 = 900000


elif fruit3 == "love":
   fruit3 = 350000


elif fruit3 == "rubber":
   fruit3 = 400000


elif fruit3 == "barrier":
   fruit3 = 400000


elif fruit3 == "magma":
   fruit3 = 1100000


elif fruit3 == "quake":
   fruit3 = 1100000


elif fruit3 == "buddha":
   fruit3 = 2300000


elif fruit3 == "string":
   fruit3 = 1600000


elif fruit3 == "phoenix":
   fruit3 = 1700000


elif fruit3 == "portal":
   fruit3 = 2200000


elif fruit3 == "rumble":
   fruit3 = 2450000


elif fruit3 == "paw":
   fruit3 = 1750000


elif fruit3 == "blizzard":
   fruit3 = 2700000


elif fruit3 == "gravity":
   fruit3 = 2600000


elif fruit3 == "dough":
   fruit3 = 4400000


elif fruit3 == "shadow":
   fruit3 = 3300000


elif fruit3 == "venom":
   fruit3 = 4000000


elif fruit3 == "control":
   fruit3 = 2600000


elif fruit3 == "spirit":
   fruit3 = 2500000


elif fruit3 == "dragon":
   fruit3 = 4200000


elif fruit3 == "leopard":
   fruit3 = 7800000


# fruit 4
if fruit4 == "blank":
   fruit4 = 0




elif fruit4 == "kilo":
   fruit4 = 7500




elif fruit4 == "spin":
   fruit4 = 10000


elif fruit4 == "chop":
   fruit4 = 100000


elif fruit4 == "spring":
   fruit4 = 20000


elif fruit4 == "bomb":
   fruit4 = 30000


elif fruit4 == "smoke":
   fruit4 = 200000


elif fruit4 == "spike":
   fruit4 = 100000


elif fruit4 == "flame":
   fruit4 = 450000


elif fruit4 == "falcon":
   fruit4 = 200000


elif fruit4 == "ice":
   fruit4 = 800000


elif fruit4 == "sand":
   fruit4 = 750000


elif fruit4 == "dark":
   fruit4 = 1000000


elif fruit4 == "revive":
   fruit4 = 450000


elif fruit4 == "diamond":
   fruit4 = 550000


elif fruit4 == "light":
   fruit4 = 900000


elif fruit4 == "love":
   fruit4 = 350000


elif fruit4 == "rubber":
   fruit4 = 400000


elif fruit4 == "barrier":
   fruit4 = 400000


elif fruit4 == "magma":
   fruit4 = 1100000


elif fruit4 == "quake":
   fruit4 = 1100000


elif fruit4 == "buddha":
   fruit4 = 2300000


elif fruit4 == "string":
   fruit4 = 1600000


elif fruit4 == "phoenix":
   fruit4 = 1700000


elif fruit4 == "portal":
   fruit4 = 2200000


elif fruit4 == "rumble":
   fruit4 = 2450000


elif fruit4 == "paw":
   fruit4 = 1750000


elif fruit4 == "blizzard":
   fruit4 = 2700000


elif fruit4 == "gravity":
   fruit4 = 2600000


elif fruit4 == "dough":
   fruit4 = int(4400000)


elif fruit4 == "shadow":
   fruit4 = 3300000


elif fruit4 == "venom":
   fruit4 = 4000000


elif fruit4 == "control":
   fruit4 = 2600000


elif fruit4 == "spirit":
   fruit4 = 2500000


elif fruit4 == "dragon":
   fruit4 = 4200000


elif fruit4 == "leopard":
   fruit4 = 7800000


print(fruit1, "+", fruit2, "+", fruit3, "+", fruit4, "=", fruit1 + fruit2 + fruit3 + fruit4)

\$\endgroup\$
3
  • \$\begingroup\$ Read the Style Guide for Python Code. \$\endgroup\$
    – greybeard
    Feb 21 at 8:00
  • 1
    \$\begingroup\$ Welcome to Code Review! The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ Feb 21 at 9:12
  • 1
    \$\begingroup\$ The new title should sescribe beter, I hope to have understood it right. \$\endgroup\$
    – convert
    Feb 21 at 19:31

2 Answers 2

3
\$\begingroup\$

zomg, my eyes are bleeding, it is copy-n-paste, copy-n-paste, make it stop!

Two bits of advice:

  1. define a helper() already
  2. prefer a dict mapping over endless if / elif / elif / elif ...

You require a mapping from string to integer. Model it along these lines:

def price_of(fruit_name: str) -> int:
    ...

What goes in the body? Define a dict like this:

name_to_price = {
    "blank":    0,
    "kilo":  7500,
    "spin": 10000,
    ...
}

Then your helper can return name_to_price[fruit_name].

You wish to solicit user input several times. Good. Then call the helper several times, as well. DRY.

\$\endgroup\$
4
  • \$\begingroup\$ i’m not sure what a helper() is \$\endgroup\$
    – Lhinger
    Feb 21 at 8:07
  • 1
    \$\begingroup\$ @Lhinger helper() means a helper function, i.e. price_of(...) in this case. Also, staying with DRY, fruit1 to fruit4 screams for using a list instead and iterating over it. \$\endgroup\$
    – lukstru
    Feb 21 at 13:25
  • \$\begingroup\$ im starting to figure it out, im confused on how to use return name_to_price[fruit_name] \$\endgroup\$
    – Lhinger
    Feb 21 at 16:16
  • \$\begingroup\$ nvm i think i figured it out! tysm \$\endgroup\$
    – Lhinger
    Feb 21 at 16:20
2
\$\begingroup\$

Code does tend to get long and bit messy when one's approach to repeating things consists of copying and pasting code. We've all been there, but there are better ways

Those if/elif/else blocks just look at a variable, check if it matches a certain constant, and outputs another constant if it does. Essentially, it's a big lookup table. But, conveniently enough, that kind of lookup table can be stored in a variable and used multiple times (in Python, such values are known as dicts). So, you could instead define, use, and re-use that lookup table like:

fruit_value_table = {
    "blank": 0,
    "kilo": 7500,
    ...: ... # and so on
}

fruit_1_value = fruit_value_table[fruit_1]
fruit_2_value = fruit_value_table[fruit_2]
...

Note that I introduced new variables fruit_1_value, fruit_2_value, etc. The original code re-used the same variable instead. Which works, but it means that some of the time fruit_1 is the name of the first fruit, and the value of the first fruit at other times. Which can get confusing to keep track of, so it's usually considered good practice to not re-use the same variable for different values (creating a new variable is cheap anyway)

But beyond that, a lot of the time when there are several variables with names like thingy_1, thingy_2, thingy_3, etc, it's often more useful to think of those variables not as several different values, but as a single bunch of values. So instead of several variables, you'd have a single list, and use loops to handle that whole list instead of manually handling each value in it. For example, you might do:

fruit_value_table = ... # See above
how_many_fruits = 4

fruit_names = []
for _ in range(how_many_fruits):
    fruit_names.append(input("Enter a fruit:")) # Add a new fruit to the end of the fruit list

# There are neater ways to do this part, but let's keep things simple for now
fruit_values = []
fruit_values_as_text = []
for fruit_name in fruit_names:
    value = fruit_value_table[fruit_name]
    fruit_values.append(value)
    fruit_values_as_text.append(value)

left_hand_side = " + ".join(fruit_values_as_text)

print(left_hand_side + " = " + sum(fruit_values))

This lets you easily change how many fruits you want the user to pass, and even gives you the option of just letting the user pass however many fruits they feel like - just collect them into a list, and then work with the list, because a list is a list no matter how long or short it is

Speaking of user input, it's often useful to remember that the user might give you some input you don't expect. Imagine if a user accidentally asks for a klio fruit instead of a kilo fruit, for example. What does the program do then, and is that what you think it should do?

Oh, and finally, you can just delete that add function, since you're not using it

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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