1
votes

I have the following function, and PyCharm is alerting me on the elif statements about "simplify chained comparison". The code works and I am getting the object I want, just wondering about the warning and how I can make it better?

     def preferred_contacts(self):
         x = random.randint(0, 100)
         email = u'E'
         text = u'M'
         phone = u'P'
         letter = u'L'
         none = u'N'

         if x < 25:
            return email
         elif x >= 26 and x <= 50:
            return text
         elif x >= 51 and x <= 75:
            return phone
         elif x >= 76 and x <= 100:
            return letter
         else:
            return none
2
You could certainly remove all of the x >= comparisons, since by virtue of reaching elif it's already been shown to not match the earlier conditions - mhlester
Also, you'll be glad to learn that : elif 76 <= x <= 100: would you what you expect it to do. - SylvainD
Really you don't need the and ops either; elif 26 <= x <= 50 and so on... - Drewness
Also, this will never return none, as you've bounded x at 100, and 100 will return letter. - Silas Ray

2 Answers

1
votes

@mhlester should get the credit for noting that you can drop the >= clauses from the conditions as they are already implicit since you are using elif. However, you could also condense things more if you wanted by putting your data in a tuple then indexing in to it.

return ('E', 'M', 'P', 'L', 'N')[x / 25] # This assumes x has an upper bound of 124 or less.

Of course, in this particular instance, you could make your life even simpler.

return random.choice(('E', 'M', 'P', 'L', 'N'))
1
votes

Simplified chained calls for much cleaner code. See below

def preferred_contacts(self):
x = random.randint(0, 100)
email = u'E'
text = u'M'
phone = u'P'
letter = u'L'
none = u'N'

if x < 25:
    return email
elif 26 <= x <= 50: # reads as "x is between 26 and 50, inclusive
    return text
elif 51 <= x <= 75: # reads as "x is between 51 and 75, inclusive
    return phone
elif 76 <= x <= 100: # reads as "x is between 76 and 100, inclusive
    return letter
else:
    return none