how to simplify boolean logic in if else statement?

Question

I have 4 variables, some of them will be True and False, and for each combinations, i will have to call one or few functions. i am currently using a if else statement for each case and i would like to know if there is a nicer way to get the same result with a dictionary or something else.

Thank you

here is my code :

    if (self.cpe_ip and self.cpe_passwd) and self.phone and self.pppoe:
        print "launch ISP portal, modem and radius"
        if self.isp() == "east":
            self.launchbell()
        else:
            self.launchtelus()
        print 'check modem...'
        self.modemstatus()
        radius = sgp_radius.Radius(self.pppoe)
        print 'check radius logs...'
        self.data = radius.sgp()

        self.radius_save()

        #exit(0)
    elif (self.cpe_ip and self.cpe_passwd) and not self.phone and not self.pppoe:
        print "launch modem test only"
        self.modemstatus()


        #exit(0)
    elif not(self.cpe_ip and self.cpe_passwd) and self.phone and not self.pppoe:
        #print "only  Bell portal"
        if self.isp() == "east":
            self.launchbell()
        else:
            self.launchtelus()

    elif (self.cpe_ip and self.cpe_passwd) and not self.phone and self.pppoe:
        #print "launch modem and radius test."
        self.modemstatus()

        radius = sgp_radius.Radius(self.pppoe)
        print 'check radius logs...'
        self.data = radius.sgp()
        self.radius_save()

    elif not(self.cpe_ip and self.cpe_passwd) and not self.phone and self.pppoe:
        #print "only radius tests."
        radius = sgp_radius.Radius(self.pppoe)
        self.data = radius.sgp()

        self.radius_save()

    elif not(self.cpe_ip and self.cpe_passwd) and self.phone and self.pppoe:
        print "bell and radius tests."
        if self.isp() == "east":
            self.launchbell()
        else:
            self.launchtelus()

        radius = sgp_radius.Radius(self.pppoe)
        print 'check radius logs...'
        self.data = radius.sgp()
        self.radius_save()

    elif (self.cpe_ip and self.cpe_passwd) and self.phone and not self.pppoe:
        #print "launch modem and bell tests."
        if self.isp() == "east":
            self.launchbell()
        else:
            self.launchtelus()
        self.modemstatus()

    else:
        #print "test bell only"
        #launchbell(self.phone)
        exit(0)

jonrsharpe · Accepted Answer · 2014-06-26 12:56:33Z

5

One way to simplify it would be to recognise and factor out the duplication:

if self.phone:
    if self.isp() == "east":
        self.launchbell()
    else:
        self.launchtelus()

if self.cpe_ip and self.cpe_passwd:
    print 'check modem...'
    self.modemstatus()

if self.pppoe:
    radius = sgp_radius.Radius(self.pppoe)
    print 'check radius logs...'
    self.data = radius.sgp()
    self.radius_save()

edited Jun 26, 2014 at 12:56

answered Jun 26, 2014 at 7:26

jonrsharpe

123k31 gold badges278 silver badges488 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

neonet Over a year ago

ok thks for your quick answer, it means that i will have to use a for loop to test every element and launch the function if one of them is True...

jonrsharpe Over a year ago

@neonet I don't quite follow your comment. These three separate if clauses don't need to be looped over, you can put them as I show them; one after another in your (I'm guessing) test function.

GreenAsJade Over a year ago

What if he wants to print the currently-commented out strings depending on the different combinations? (Or, more generally, if tomorrow the logic changes and there is less duplication)

jonrsharpe Over a year ago

@GreenAsJade I would probably handle the strings separately, if really needed, before reaching if self.phone:. If the logic changes fundamentally (i.e. not just toggling three independent tests) then a completely different approach would be required, but that's hard to answer in the general case.

GreenAsJade Over a year ago

Yeah - if the final application is truly as simple as your simplification shows, hard to see why you would do anything else than what you did :)

GreenAsJade · Accepted Answer · 2014-06-26 22:47:18Z

2

You implied in your question that some sort of lookup table for the logic might be the way to go, and there are many ways you could make something like that.

One way would be, as you say, with a dict, as shown below.

This way results in rather more complex looking code than the nice looking "simplify and factor out duplication" solution for your particular example, but this method extends more easily to the case where you have more different logic for the different cases... I'm not saying it's "better" ... just a different option!

def launch_isp_portal_and_radius():
   print "launch ISP portal, modem and radius"
   # etc

# etc

DecisionTable = {
 # cpe_ip cpe_passwd phone ppoe
   (True,  True,  True,  True ) : launch_isp_portal_and_radius,
   (True,  True,  False, False) : launch_modem_test_only,
   (False, False, True,  False) : only_Bell_portal,
   (False, True,  True,  False) : only_Bell_portal,
   (True,  False, True,  False) : only_Bell_portal,
   (True,  True,  False, True ) : launch_modem_and_radius_test,
   (False, False, False, False) : only_radius_tests,
   (False, True,  False, False) : only_radius_tests,
   (True,  False, False, False) : only_radius_tests,
   (False, False, True,  True ) : bell_and_radius_tests
   (False, True,  True,  True ) : bell_and_radius_tests,
   (True,  False, True,  True ) : bell_and_radius_tests,
   (True,  True,  True,  False) : launch_modem_and_bell_tests
}

action = DecisionTable.get((self.cpe_ip, self.cpe_passwd, self.phone, self.ppoe))

if action:
   action()
else:
   raise StandardError("Whoa, internal logic error!")

(And of course there are other ways to make a decision table - an array is another obvious option. The problem with an array is making the static declaration of it look readable for the logic it represents...)

(Edit: made the dict indexed on tuple, as per jons suggestion)

edited Jun 26, 2014 at 22:47

answered Jun 26, 2014 at 12:21

GreenAsJade

14.7k11 gold badges68 silver badges103 bronze badges

5 Comments

jonrsharpe Over a year ago

This is actually the other approach I had in my head! Two comments: 1. a four-tuple of booleans would probably be a better key (inputs = tuple(map(bool, (cpe_ip, cpe_passwd, phone, ppoe)))); and 2. I would split the sub-tests (e.g. launch ISP) into sub-functions. I would also be tempted to use try: DecisionTable[inputs](); except KeyError: ....

GreenAsJade Over a year ago

Yeah - totally agree about factoring out common subfunctions. But OH MY GOD ... I had no idea you can index a dict with a tuple! That is heaps more elegant ... I will rewrite thusly!

jonrsharpe Over a year ago

Anything that can be hashed and equality-compared (inlcuding custom classes that implement __hash__ and __eq__) can be a key. Note that you don't define choices, so your last line is a NameError.

neonet Over a year ago

Thank you for all your comments, i like the way with the dictionary GreenAsJade, i was wondering how to implement it with a dictionary... and the Jons's way seems very elegant i have to try that out.

GreenAsJade Over a year ago

@jonrsharpe damn no regression test suite ;) :D neonet: the standard way of saying that you like a solution is to upvote it ;)

Collectives™ on Stack Overflow

how to simplify boolean logic in if else statement?

2 Answers 2

5 Comments

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related