Army strength based on level and modifiers

Question

Right so I was doing a quick project, and sought a quick way to finish one section of my Python code. I handed it in, but I'm well aware that it's bad code, but I was wondering if there was a good way to condense this code into fewer lines.

def nGen_army(level, strength, weakness):
 if weakness == 1:
 if level == 0:
 army = round(random.randint(1000,5000),4)
 return army
 if level == 1:
 army = round(random.randint(10000,25000),4)
 return army
 if level == 2:
 army = round(random.randint(50000,75000),4)
 return army
 if level == 3:
 army = round(random.randint(100000,200000),4)
 return army
 if level == 4:
 army = round(random.randint(200000,500000),4)
 return army
 if strength == 1:
 if level == 0:
 army = round(random.randint(10000,25000),4)
 return army
 if level == 1:
 army = round(random.randint(50000,100000),4)
 return army
 if level == 2:
 army = round(random.randint(100000,300000),4)
 return army
 if level == 3:
 army = round(random.randint(300000,800000),4)
 return army
 if level == 4:
 army = round(random.randint(800000,1100000),4)
 return army
 else:
 if level == 0:
 army = round(random.randint(5000,10000),4)
 return army
 if level == 1:
 army = round(random.randint(25000,50000),4)
 return army
 if level == 2:
 army = round(random.randint(75000,150000),4)
 return army
 if level == 3:
 army = round(random.randint(150000,250000),4)
 return army
 if level == 4:
 army = round(random.randint(300000,900000),4)
 return army

Answer 1

Here's one way to do it:

from random import randint
def n_gen_army(level, strength, weakness, params=dict_of_dicts):
 """Returns random floats according to ranges specified by params."""
 if strength:
 key = 'strength'
 if weakness:
 key = 'weakness'
 else:
 key = 'default'
 lower_bound, upper_bound = params[key][level]
 return round(randint(lower_bound, upper_bound), 4)

---

weakness_dict = {
 0: (1000, 5000),
 1: (10000, 50000),
 2: (50000, 75000),
 3: (100000, 200000),
 4: (200000, 500000),
 }
strength_dict = {
 0: (10000, 25000),
 1: (50000, 100000),
 2: (100000, 300000),
 3: (100000, 300000),
 4: (800000, 1100000),
 }
default_dict = {
 0: (5000, 10000),
 1: (25000, 50000),
 2: (75000, 150000),
 3: (150000, 250000),
 4: (300000, 900000),
 }
dict_of_dicts = {'weakness': weakness_dict, 
 'strength': strength_dict, 
 'default': default_dict
 }
>>> n_gen_army(2, 0, 0)
115061.0

Some other notes:

1. In your original code, if strength and weakness are both 1, then only the setting for weakness matters. My code preserves that, but is that behavior really what is intended? Or did you want elif instead of your second top-level if?

2. Why are you using round? Do you really want floats returned instead of integers? If you want integers, don't use round. If you want integers rounded to the nearest 10 or 100, why not generate random integers in a 10-fold (or 100-fold) reduced range and then multiply be 10 (or 100)?

3. Avoid capitalization in function names if you can. PEP8 and all that. All-lowercase (for functions) is good python style.

4. You're only using one function from random, so from random import randint is better than loading in the whole module with import random.

5. You seem to be using weakness and strength as boolean variables so if weakness: is better than if weakness == 1.

• Curt F.: A list of tuples also works fine I think, but assumes that level will always be a non-negative integer. If this code wants extensions to level = 'infinite' or level='demigod' or even level=-1, then a dictionary is better. Whether that's a valid concern or not depends on the end application.
• Curt F.: Yes, reversing the order was intentional so that if weakness: could "overwrite" if strength: since I am not returning early. That bit of the code is messy in both the original and in my version. I'm sure there's a better way to handle that logic.
• Habeeb: Very late reply, but this did help a lot. I was unfamiliar with dictionaries (not sure why, they seem to be incredibly useful). I wasn't actually using weakness and strength as boolean - the number corresponds to a different section of the fictional armed forces (army, navy, air force). With some tweaking I managed to make it fit by changing the original function from which all the other parameters were generated. Again, thank you!

Answer 2

Here is declaration of a three dimensional list of list of list (of ... :) ), where the first dimension is based upon your variables, the second dimension is the level (I only added 3, not 5 as you have), and the last dimension is for the two different randint-parameters.

Have also added a print statement to list which combination is used for debug purposes, and some actual calls to the method.

from random import randint
# 1st: weakness=0, strength=1, or neither=2
# 2nd: =level
# 3rd: limits of randint - upper=0, lower=1 
riPar = [
 [ [1000, 5000], [10000, 25000], [50000, 75000] ], # Weakness 
 [ [10000, 25000], [50000, 100000], [100000, 300000] ], # Strength 
 [ [13, 14], [15,16], [17, 18] ] # none?
 ] 
def nGen_army(level, strength, weakness):
 if weakness == 1:
 idx = 0
 elif strength == 1:
 idx = 1
 else:
 idx = 2
 print ("riPar[{0}][{1}]: {2}".format(idx, level, riPar[idx][level]))
 return round( randint( riPar[idx][level][0], riPar[idx][level][1] ), 4)
print nGen_army(1, 0, 1)
print nGen_army(2, 0, 0)

Left the complete initialisation of the riPar list to you. Enjoy!

Output from a run:

riPar[0][1]: [10000, 25000]
11211.0
riPar[2][2]: [17, 18]
18.0

Code review

The key point for this simplification is to recognise the pattern you were using (i.e. always calling the randint()), remove unneccessary variable (army), and introduce a multi-dimensional list (riPar) to keep the actual difference in the otherwise equal code segments.

• mkrieger1: In Python, expressions in parentheses, square brackets or curly braces can be split over more than one physical line without using backslashes, so if you put the [ on the same line as riPar =, neither backslashes nor comments are required.
• Habeeb: Gotcha! Now it's just to generate the navy and the airforce, hopefully without so much repetition this time!