How to write shorter Python code with several if and else statements?

user_8519380 0
Viewed 77

How can you optimize the following code and write less?

  class CleanItem():

     def process_item(self, item, spider):

        PSCV = str(item['page_source_canonical']).split("'")[1]

        if PSCV != "":
            if PSCV != item['page_source']:
                item['page_source_canonical_is_itself'] = False
            else:
                item['page_source_canonical_is_itself'] = True
        else:
                item['page_source_canonical_is_itself'] = True

    
        return item

First it checks if it is empty. If it is empty, it is true. If it is not empty it should be checked and if it is the same then again it is true. otherwise it is false.

python
3 Answers

You wrote

        if PSCV != "":
            if PSCV != item['page_source']:
                item['page_source_canonical_is_itself'] = False
            else:
                item['page_source_canonical_is_itself'] = True
        else:
                item['page_source_canonical_is_itself'] = True

You want

        item['page_source_canonical_is_itself'] =  PSCV in (
            '', item['page_source'])

That's DRY, and very clearly spells out Author's Intent that we shall assign True upon matching either value, else False.

Style nit: PEP 8 asks that you spell it pscv, lowercase.

Consider deleting the unused spider parameter.

user_84311110

Yes, you could simplify

if PSCV != "":
    if PSCV != item['page_source']:
        item['page_source_canonical_is_itself'] = False
    else:
        item['page_source_canonical_is_itself'] = True
else:
    item['page_source_canonical_is_itself'] = True

to

if PSCV != "" and PSCV != item['page_source']:
    item['page_source_canonical_is_itself'] = False
else:
    item['page_source_canonical_is_itself'] = True

or even further to

condition = PSCV != "" and PSCV != item['page_source']
item['page_source_canonical_is_itself'] = not condition
user_123637500 
 if PSCV != "":
        if PSCV != item['page_source']:
            item['page_source_canonical_is_itself'] = False
        else:
            item['page_source_canonical_is_itself'] = True
    else:
            item['page_source_canonical_is_itself'] = True

If I wanted to refactor the code above, two options

  1. The most explanatory (not necessarily the shortest)

      if (PSCV == ""):
        item['page_source_canonical_is_itself'] = True
  elif (PSCV == item['page_source']):
        item['page_source_canonical_is_itself'] = True
  else:
        item['page_source_canonical_is_itself'] = False

  2. The shorter - ternary

     item['page_source_canonical_is_itself'] = ( (PSCV == "") or (PSCV == item['page_source']) )? true: false
user_18695620
Related