Cleaning and simplifying function that returns a number based on certain values

Question

I have a rather clunky function that returns a number based on certain values:

local = {'abs154': '4'}
w12,sv2,sv4,sv6,sv8,sv10,sv12=75,95,110,104,101,110,116 
supers = [["5", w12], ["6", w12], ["7", w12], ["8", w12], ["16", w12], ["17", w12], ["18", w12], ["9", sv2], ["11", sv2], ["12", sv2], ["13", sv2], ["14", sv2], ["15", sv2], ["19", sv4], ["23", sv4], ["24", sv4], ["25", sv4], ["26", sv4], ["28", sv6], ["29", sv6], ["30", sv6], ["31", sv6], ["32", sv6], ["33", sv6], ["35", sv8], ["36", sv8], ["37", sv8], ["38", sv8], ["39", sv8], ["40", sv8], ["41", sv8], ["42", sv8], ["43", sv8], ["44", sv8], ["45", sv8], ["46", sv8], ["47", sv8], ["48", sv8], ["49", sv8], ["50", sv8], ["52", sv10], ["53", sv10], ["55", sv10], ["57", sv10], ["58", sv10], ["59", sv10], ["60", sv10], ["61", sv10], ["62", sv10], ["63", sv10], ["64", sv10], ["65", sv10], ["66", sv10], ["68", sv2], ["71", sv12], ["72", sv12], ["73", sv12], ["74", sv12], ["75", sv12], ["76", sv12], ["77", sv12], ["78", sv12], ["79", sv12], ["80", sv12], ["81", sv12], ["82", sv12], ["83", sv12], ["84", sv12]] 
def Server(group): 
 try: sn = local[group] 
 except KeyError: 
 group = group.replace("-", "q") 
 fnv = float(int(group[0:min(5, len(group))], 36)) 
 lnv = group[6: (6 + min(3, len(group) - 5))] 
 if(lnv): 
 lnv = float(int(lnv, 36)) 
 lnv = max(lnv,1000) 
 else: 
 lnv = 1000 
 num = (fnv % lnv) / lnv 
 maxnum = sum(map(lambda x: x[1], supers)) 
 cumfreq = 0 
 sn = 0 
 for wgt in supers: 
 cumfreq += float(wgt[1]) / maxnum 
 if(num <= cumfreq): 
 sn = int(wgt[0]) 
 break 
 return "s" + str(sn) + ".website.com"

This all looks rather unnecessary. I would like to make it shorter and cleaner if possible.

• user2602977: sorry about the lack of code blocks. seems a bit different than stack
• Appleshell: Just click the code brackets in the editor toolbar while having the code selected
• user2602977: edited. was referred here by stack, yet no luck thus far
• jaynp: @user2602977 Have you considered breaking this down into subfunctions with suggestive names?
• 200_success: What is the motivation for this code? I suspect that it's excessively complicated because of an XY problem.

Answer 1

some style issues to consider:

1) it's conventional to use ALL_CAPS for constants such as 'supers' and w1, etc.

2) for long term maintenance, it's also a good idea to use longer more descriptive names - not only is this hard to parse for the uninitiated, it's also prone to hard-to-catch typos which will be hard to spot. It's especially bad when you do multiple assignments on a single line.

3) supers could be a dictionary

4) you can simplify the range logic by excepting for incomplete groups:

<snip>
enter code here
group = group.replace("-", "q") 
fnv, lnv = None
try: 
 fnv = group[:5] 
 lnv = group[6:9]
except IndexError:
 raise ValueError "group string %s is malformed"
fn_value = int(fnv, 36)
ln_value = max(int(lnv, 36), 1000) or 1000

5) maxNum is always the same, it should be a constant or a cached module-level field:

informative_name = sum(map(lambda x: x[1], supers)) 

at the top will execute at module load and wont need to be recalced for every call.

Answer 2

• Extract two functions from the except clause: one that extracts num from group, and one that looks up sn from super
• Precomputing cumfreq into super would allow easy lookup with help from the bisect module.
• group[0:min(5, len(group))] is same as group[:5] and group[6: (6 + min(3, len(group) - 5))] same as group[6:9]
• There are redundant type conversions: lnv is made float but possibly replaced by the integer 1000, and sn goes from str to int to str.