Python code optimization with regexes

Question

I'm building a web app with the purpose to register all the calls made by users. A call can have a service, number called and a cost. The code provided below is currently working (with the numbers turned into regexes), but it's somewhat of a mess and I would like to optimize it. Since this code requires to be exactly in this order to work, when I used a dictionary to store the regex expressions and services, it didn't work. Here is my current code with the regex used:

  if (service=='R'):
        return "ROAMING"
    elif (service=='V  O') or ((service=='S') and (float(cost)==0.0)):
        return "ONNET"
    elif (service=="") and (nr_called==""):
        return "INTERNET"
    elif (service=='I')or (service=='ROAMING - MMS'):          
        return "OTHERSERV"        
    elif (service=='Internet') or (service=='WAP') or service==('BLACKBERRY.NET') or (service=='ROAMING - INTERNET')or (service=='ROAMING - BLACKBERRY'):
        return "INTERNET"
    elif ((nr_called[:6]=="003516" or nr_called[:6]=="003514" or nr_called[:6]=="003511" or nr_called[:6]=="003517" or nr_called[:6]=="003518") or (nr_called[0]=="6"  or nr_called[0]=="4" or nr_called[0]=="1" or nr_called[0]=="7" or nr_called[0]=="8"))  and (service!="V  O"):
        return "OTHERSERV"    
    elif (service=='Vi F') or (service=='Si'):
        return "INTERNATIONAL"
    elif  (nr_called[0]=="9" or nr_called[:6]=="003519") and (service=="Vp F") and (float(cost)>0) :           
        return "INS"   
    elif (nr_called[:9]=="003519220" or  nr_called[:8]=="00351924" or nr_called[:8]=="00351925" or nr_called[:8]=="00351926" or nr_called[:8]=="00351927" or nr_called[:4]=="9220" or nr_called[:3]=="924" or nr_called[:3]=="925" or nr_called[:3]=="926" or nr_called[:3]=="927") :          
        return "INS"
    elif ((len(nr_called)==9) and nr_called[:2]=="96") or (nr_called[:7]=="0035196"):
        return "INS"
    elif (nr_called[:3]=="921" or  nr_called[:8]=="00351921"):
        return "91"
    elif (nr_called[:3]=="929" or  nr_called[:8]=="00351929"):
        return "93"
    elif (nr_called[:3]=="922" or  nr_called[:8]=="00351922"):
        return "OTHERSERV"
    elif (service!="Vp F") and ((nr_called[:2]=="96" ) or(nr_called[:7]=="0035196")):            
        return "INS"
    elif (len(nr_called)==7) or (service=='V O'):           
        return "ONNET"
    elif ((len(nr_called)==9) and nr_called[:2]=="91") or (nr_called[:7]=="0035191"):
        return "91"
    elif ((len(nr_called)==9) and nr_called[:2]=="93") or (nr_called[:7]=="0035193"):
        return "93"
    elif ((len(nr_called)==9) and nr_called[:1]=="2") or (nr_called[:5]=="00352"):
        return "PT"
    elif float(cost)>0:
        return "OTHERSERV"
    else:
        return "OTHERSERV"

Number regexes:

 OTHERSERV: ['(\+*0*351)?922','(\+*0*351)?[146-8]']
 96:        ['(\+*0*351)?92([4-7|20])','(\+*0*351)?96']
 93:        ['(\+*0*351)?929','(\+*0*351)?93']
 91:        ['(\+*0*351)?921','(\+*0*351)?91']
 PT:        ['(\+*0*351)?2']

I have been dwelling with the optimization for quite a while and can't figure out how can I structure this in an optimized and maintainable way, so any help is very appreciated.

Put the sequence of patterns in a list, then you control/know the order. Presumably you can normalize the numbers to start with either 0 or + for exactly 9 digits, then the number pattern could always be applied like "^00351", the most difficult thing is the test of cost=0 or >0, but as the last test for cost>0 doesn't affect the result, you could always add a flag to check the cost==0, used only for the service=='S' test. However a table isn't necessarily easier to maintain. Why do you use elif when each test includes a return statement? — DisappointedByUnaccountableMod
– DisappointedByUnaccountableMod, Commented Oct 2, 2015 at 12:04

ryachza · Accepted Answer · 2015-10-02 12:06:22Z

1

I'm not sure I understand what you're referring to about the regexes, but one option to clean up the if/elif list would be to use a list of result/check tuples:

actions = [
  ('ROAMING'  , lambda service,nr_called,cost: service=='R'),
  ('ONNET'    , lambda service,nr_called,cost: service in ('V  O','S') and float(cost)==0.0),
  ('INTERNET' , lambda service,nr_called,cost: service=='' and nr_called==''),
  ('OTHERSERV', lambda service,nr_called,cost: service in ('I','ROAMING - MMS'))

  # fallthrough
  ('OTHERSERV', lambda service_nr_called,cost: True)
]

for value,check in actions:
  if check(service,nr_called,cost):
    return value

# fallthrough
return 'OTHERSERV'

One or the other items labeled "fallthrough" would be needed.

answered Oct 2, 2015 at 12:06

ryachza

4,54020 silver badges28 bronze badges

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

Comments

DisappointedByUnaccountableMod · Accepted Answer · 2015-10-02 14:58:54Z

I was thinking of a solution which is more completely data/table-driven than @ryachza suggestion, for example:

import re

def oldcategorize( service, nr_called, cost):
    if (service=='R'):
        return "ROAMING"
    elif (service=='V  O') or ((service=='S') and (float(cost)==0.0)):
        return "ONNET"
    elif (service=="") and (nr_called==""):
        return "INTERNET"
    elif (service=='I')or (service=='ROAMING - MMS'):          
        return "OTHERSERV"        
    elif (service=='Internet') or (service=='WAP') or service==('BLACKBERRY.NET') or (service=='ROAMING - INTERNET')or (service=='ROAMING - BLACKBERRY'):
        return "INTERNET"
    elif ((nr_called[:6]=="003516" or nr_called[:6]=="003514" or nr_called[:6]=="003511" or nr_called[:6]=="003517" or nr_called[:6]=="003518") or (nr_called[0]=="6"  or nr_called[0]=="4" or nr_called[0]=="1" or nr_called[0]=="7" or nr_called[0]=="8"))  and (service!="V  O"):
        return "OTHERSERV"    
    elif (service=='Vi F') or (service=='Si'):
        return "INTERNATIONAL"
    elif  (nr_called[0]=="9" or nr_called[:6]=="003519") and (service=="Vp F") and (float(cost)>0) :           
        return "INS"   
    elif (nr_called[:9]=="003519220" or  nr_called[:8]=="00351924" or nr_called[:8]=="00351925" or nr_called[:8]=="00351926" or nr_called[:8]=="00351927" or nr_called[:4]=="9220" or nr_called[:3]=="924" or nr_called[:3]=="925" or nr_called[:3]=="926" or nr_called[:3]=="927") :          
        return "INS"
    elif ((len(nr_called)==9) and nr_called[:2]=="96") or (nr_called[:7]=="0035196"):
        return "INS"
    elif (nr_called[:3]=="921" or  nr_called[:8]=="00351921"):
        return "91"
    elif (nr_called[:3]=="929" or  nr_called[:8]=="00351929"):
        return "93"
    elif (nr_called[:3]=="922" or  nr_called[:8]=="00351922"):
        return "OTHERSERV"
    elif (service!="Vp F") and ((nr_called[:2]=="96" ) or(nr_called[:7]=="0035196")):            
        return "INS"
    elif (len(nr_called)==7) or (service=='V O'):           
        return "ONNET"
    elif ((len(nr_called)==9) and nr_called[:2]=="91") or (nr_called[:7]=="0035191"):
        return "91"
    elif ((len(nr_called)==9) and nr_called[:2]=="93") or (nr_called[:7]=="0035193"):
        return "93"
    elif ((len(nr_called)==9) and nr_called[:1]=="2") or (nr_called[:5]=="00352"):
        return "PT"
    elif float(cost)>0:
        return "OTHERSERV"
    else:
        return "OTHERSERV"
    return "FAILED"

# table of categories with regex criteria
# special treatment of first character of a pattern "-" means NOT matching
# note cost is coverted to standardized format using str() for matching
#      service                 number       cost   category
categories = [
    #    if (service=='R'):
    #        return "ROAMING"
     ["^R$"                       ,""         ,""         ,"ROAMING"]
    #    elif (service=='V  O') or ((service=='S') and (float(cost)==0.0)):
    #        return "ONNET"
    ,["^V  O$"                    ,""         ,""         ,"ONNET"]
    ,["^S$"                       ,""         ,"^0.0$"    ,"ONNET"]
    # elif (service=="") and (nr_called==""):
    #   return "INTERNET"
    ,["^$"                      ,"^$"       ,""         ,"INTERNET"]
    #    elif (service=='I')or (service=='ROAMING - MMS'):          
    #        return "OTHERSERV"        
    ,["^I$"                       ,""         ,""         ,"OTHERSERV"]
    ,["^ROAMING - MMS$"           ,""         ,""         ,"OTHERSERV"]
    #   elif (service=='Internet') or (service=='WAP') or service==('BLACKBERRY.NET') or (service=='ROAMING - INTERNET')or (service=='ROAMING - BLACKBERRY'):
    #       return "INTERNET"
    ,["^Internet"                ,""         ,""         ,"INTERNET"]
    ,["^WAP"                     ,""         ,""         ,"INTERNET"]
    ,["^BLACKBERRY.NET"          ,""         ,""         ,"INTERNET"]
    ,["^ROAMING - INTERNET"      ,""         ,""         ,"INTERNET"]
    ,["^ROAMING - INTERNET"      ,""         ,""         ,"INTERNET"]
    ,["^ROAMING - INTERNET"      ,""         ,""         ,"INTERNET"]
    ,["^ROAMING - BLACKBERRY"    ,""         ,""         ,"INTERNET"]
    #   elif ((nr_called[:6]=="003516" or nr_called[:6]=="003514" or nr_called[:6]=="003511" or nr_called[:6]=="003517" or nr_called[:6]=="003518") or (nr_called[0]=="6"  or nr_called[0]=="4" or nr_called[0]=="1" or nr_called[0]=="7" or nr_called[0]=="8"))  and (service!="V  O"):
    #       return "OTHERSERV"
    ,["-^V  O"                   ,"^00351[14678]"    ,""         ,"OTHERSERV"]
    ,["-^V  O"                   ,"^[15678]"         ,""         ,"OTHERSERV"]
    #    elif (len(nr_called)==7) or (service=='V O'):           
    #        return "ONNET"
    ,[""                        ,"^.......$"    ,""         ,"ONNET"]
    ,["^V O$"                     ,""             ,""         ,"ONNET"]
    #   ((len(nr_called)==9) and nr_called[:2]=="91") or (nr_called[:7]=="0035191"):
    #       return "91"
    ,[""                        ,"^91.......$"  ,""         ,"91"]
    ,[""                        ,"^0035191"     ,""         ,"91"]
]

def newcategorize(service, nr_called, cost ):
    for servpat,numpat,costpat,res in categories:
        print servpat,numpat,costpat,res
        if (servpat[0]=="-" and not re.match(servpat[1:],service)) or re.match(servpat,service):
            if (numpat[0]=="-" and not re.match(numpat[1:],nr_called)) or re.match(numpat,nr_called):
                if (costpat[0]=="-" and not re.match(costpat[1:],str(cost))) or re.match(costpat,str(cost)):
                    return res
    result = "FAILED to find %s %s %s"%(service,nr_called, cost)
    # print result
    return result

testdata = [
    ["x","012345",0.0]
    ,["S","123",0.0]
    ,["NOT V  O","003517",0.0]
]

for s,n,c in testdata:
    oc = oldcategorize(s,n,c)
    nc = oldcategorize(s,n,c)
    if oc != nc:
        print "ERROR",s,n,c,oc,nc
    print "match",s,n,c,oc,nc

So the table contains all the data needed to drive the logic, but the logic itself is coded in the newcategorize() function, using the power of regex to do most of the hard work, with only the addition that a leading - in a pattern makes the logic look for the pattern NOT matching. Using this approach the table is more concise, whereas with @ryachza solution the table could get quite difficult to read and probably debug for the more complex lambda expressions. With my approach you just add print statements.

HTH Barny

I like this concept a lot! The obvious tradeoff is the redundancy from splitting the "or" conditions losing the coupling and having to restructure e.g. (x or y) and z to (x and z) or (y and z) allowing z to vary independently. I think this solution is certainly worth considering based on the overall context of the problem.

Collectives™ on Stack Overflow

Python code optimization with regexes

2 Answers 2

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related