How replace all these "if...elif...else" statements with something more concise?-CodePudding

Is there a way to reduce the amount of elif things in my statement? In another question I asked someone said that my code suffered from redundancy and it definitely does. I'm relatively new to python and coding in general and when I look things up it's always kind of hard to understand.

This is the block of code I'm talking about:

(If needed I can post the rest of the code)

if (random_mon_name.lower()=='normal'):
    rangen_mon_name = random.choice(mon_name_type_NORMAL)
    break
elif (random_mon_name.lower()=='fire'):
    rangen_mon_name = random.choice(mon_name_type_FIRE)
    break
elif (random_mon_name.lower()=='water'):
    rangen_mon_name = random.choice(mon_name_type_WATER)
    break
elif (random_mon_name.lower()=='grass'):
    rangen_mon_name = random.choice(mon_name_type_GRASS)
    break
elif (random_mon_name.lower()=='electric'):
    rangen_mon_name = random.choice(mon_name_type_ELECTRIC)
    break
elif (random_mon_name.lower()=='ice'):
    rangen_mon_name = random.choice(mon_name_type_ICE)
    break
elif (random_mon_name.lower()=='fighting'):
    rangen_mon_name = random.choice(mon_name_type_FIGHTING)
    break
elif (random_mon_name.lower()=='poison'):
    rangen_mon_name = random.choice(mon_name_type_POISON)
    break
elif (random_mon_name.lower()=='ground'):
    rangen_mon_name = random.choice(mon_name_type_GROUND)
    break
elif (random_mon_name.lower()=='flying'):
    rangen_mon_name = random.choice(mon_name_type_FLYING)
    break
elif (random_mon_name.lower()=='psychic'):
    rangen_mon_name = random.choice(mon_name_type_PSYCHIC)
    break
elif (random_mon_name.lower()=='bug'):
    rangen_mon_name = random.choice(mon_name_type_BUG)
    break
elif (random_mon_name.lower()=='ghost'):
    rangen_mon_name = random.choice(mon_name_type_GHOST)
    break
elif (random_mon_name.lower()=='rock'):
    rangen_mon_name = random.choice(mon_name_type_ROCK)
    break
elif (random_mon_name.lower()=='dark'):
    rangen_mon_name = random.choice(mon_name_type_DARK)
    break
elif (random_mon_name.lower()=='dragon'):
    rangen_mon_name = random.choice(mon_name_type_DRAGON)
    break
elif (random_mon_name.lower()=='steel'):
    rangen_mon_name = random.choice(mon_name_type_STEEL)
    break
elif (random_mon_name.lower()=='fairy'):
    rangen_mon_name = random.choice(mon_name_type_FAIRY)
    break
else:
    print("You didn't actually enter a type.\nTry Again.")

CodePudding user response：

I would suggest you to create a single dict, this way:

myDict = {}
myDict['ice'] = mon_name_type_ICE
# ...and so on

So that then you will access it with a small piece of code:

rangen_mon_name = random.choice(myDict[random_mon_name.lower()])

You can also add a try/except block to simulate your else:

try:
    rangen_mon_name = random.choice(myDict[random_mon_name.lower()])
except KeyError:
    print("You didn't actually enter a type.\nTry Again.")

CodePudding user response：

The root cause of there being so much redundancy in this code is the huge number of variables you have to manage.

Whenever you have a bunch of individual variables that all hold the same kind of data, like mon_name_type_FIRE and mon_name_type_WATER (I assume these are lists of names associated with that attack type), it's a strong clue that you should instead put them into a collection, like a dictionary, or a list. Not only does this reduce the number of variables to keep track of in your code, it makes it possible to access those values dynamically rather than with a ton of copy pasted code.

For example, if you took all your different mons and put them in a dict that's keyed by their attack type, instead of creating all the mon_name_type_WHATEVER variables:

mon_by_type = {
    "fire": ["charmander", "charmeleon", "charizard"],
    "water": ["squirtle", "wartortle", "blastoise"],
    # etc
}

you can now replace your giant if/elif chain with a single dictionary lookup:

try:
    random_mon = random.choice(mon_by_type[random_mon_name.lower()])
except KeyError:
    print("You didn't actually enter a type.  Try again.")

CodePudding user response：

If you are not using the variables like mon_name_type_FIRE in other areas of your code and you are able to create a "names" lookup or dictionary you will definitely want to do that. See answers by @FLAC-ZOSO and @samwise to get you started.

If however, you have all these "name" lists already defined and want to use the "appropriate" one, you might think about just getting them from locals() (or globals()).

import random

## ------------------
## You have these pre-defined and want to keep them
## as distinct variables for some reason.
## ------------------
mon_name_type_NORMAL = ["Elf", "Human"]
mon_name_type_FIRE = ["Fire Dwarf", "Cinder Elf"]
## ------------------

## ------------------
## Use a "mob" class and locals() to find the appropriate matching
## list of mob names.
## ------------------
random_mon_name = "Fire"
rmn_key = f"mon_name_type_{random_mon_name.upper()}"
name_options = locals().get(rmn_key, ["**unnamed mob**"])
## ------------------

print(random.choice(name_options))

CodePudding user response：

Construct a dictionary like choices below. Pass the attack value to get_attack_values and if the attack is in choices, pass the value, i.e. mon_name_type_ICE to the random.choice() function.

import random

choices = { 
        'ice': {'choice': 'mon_name_type_ICE'},
        'fire': { 'choice': 'mon_name_type_WATER'}
         }
       
def get_attack_value(attack):
    if attack in choices.keys():
        rangen_mon_name = random.choice(choices[attack]['choice'])
    else:
        print("You didn't actually enter a type.\nTry Again.")

get_attack_value('fire')

CodePudding user response：

So far I like @Samwise's answer the best because it makes the code "data-driven" because it not only eliminates all all those variables, it also make extending things trivial as well — but think it could be taken a little bit further by making the creation of the one dictionary itself also done that way. Here's what I mean:

from pprint import pprint
import random


MON_TYPE_NAMES = """\
    normal, Normodo, Normalia, Normew
    fire, Firaint, Heatoro, Flamala
    water, Splashoto, Oceatto, Puddlio
    grass, Weedevon, Grassimar, Leafadroon
    electric, Enetick, Electol, Litero
    ice, Snova, Icello, Colaggro
    fighting, Allphist,Lullakick,Bubblow
    poison, Poizase, Gunkamight, Aison
    ground, Groutal, Moundiflox, Umbron
    flying, Wingyu, Sparial, Flogun
    psychic, Mindswell, Thinkarva, Psymarine
    bug, Bugonite, Flyzaur, Mantile
    ghost, Phantini, Spirook, Tortasm
    rock, Storilot, Rocoalia, Sedite
    dark, Nolite, Dariotte, Nitrash
    dragon, Dracat, Wyveron, Randigon
    steel, Metalaze, Gearevo, Iromonora
    fairy, Faeream, Pixirola, Trixitrite
"""

# Create dictionary from mon type names.
name_to_choices = {v[0]: v[1:] for v in (line.replace(',', '').split()
                                    for line in MON_TYPE_NAMES.splitlines())}
pprint(name_to_choices, sort_dicts=False)
print()

# Select random mon name based on mon type.
random_mon_type = 'Ghost'
if not (choices := name_to_choices.get(random_mon_type.lower())):
    print("Unknown mon type")
else:
    rangen_mon_name = random.choice(choices)
    print(f'random {random_mon_type} name -> {rangen_mon_name}')

CodePudding user response：

This seems like you would benefit from throwing all of these into a data structure like a dictionary. Since you're just setting values to other values with perfect 1:1 mapping a dictionary was literally made for this.

random_mon_name_dict = {'normal': random.choice(mon_name_type_NORMAL), 'bug', random.choice(mon_name_type_BUG)}

But for all of them obviously.

Am I understanding correctly?