I work mostly with Python and review code on almost daily basis. In our code base formatting & linting is handled by CI jobs using black & mypy. So, we focus only on changes.

When working in a team, you already know what kind of code to expect from a certain team member. Code reviews become interesting when a new person joins the team. I say interesting, as everyone has some coding style that they use it unconsciously; for good or bad! Like I have some,

Setting the value of Optionaltype. Usually these variables are part of the signature

# I used (long back) to do

def func(a: int, b: Optional[list] = None, c: Optional[Dict] = None):

    if b is None:
       b = []
    if c is None:
       c = {}

# Instead I do

def func(a: int, b: Optional[list] = None, c: Optional[Dict] = None):

    b = b or []
    c = c or {}

Handling Simple if..elif..else (up to 3-4) statements with dict

This is simple use case where you return a string or call a func based on some value

Note: from 3.10 you should use match instead of this.

Instead of doing,

def get_number_of_wheels(vehicle: str):
    if vehicle == "car":
        wheels = 2
    elif vehicle == "bus":
        wheels = 6
    elif vehicle == "bicycle":
        wheels = 2
    else:
        raise ...

# I prefer doing, 

def get_number_of_wheels(vehicle: str):
    return {
       "car": 2,
       "bus": 6,
       "bicycle": 2
    }[vehicle]       # Raise is now KeyError

Above are couple of examples, people who review my code will have more examples.

Recently, A new developer joined my team and i noticed a pattern which I loved but I asked to change it to simple if..else case. I will show you the pattern first then give my reason for asking for change.

The code is a decorator which does something to parameters. Lets write a simple (useless) decorator which will print number of args & kwargs with which function/method was called.

def counter(is_cls_method: bool = False):
    """ print number of args & kwargs for func/method """
    def outer(f):
        def inner(*args, **kwargs):
            args_cnt = len(args)
            kwargs_cnt = len(kwargs)
            print(f"{f.__name__} called with {args_cnt=} & {kwargs_cnt=}")
            return f(*args, **kwargs)
        return inner
    return outer

@counter()
def test1(a, b, c):
    pass

class A:
    @counter(is_cls_method=True)
    def test1(self, a, b, c):
        pass


print("function")
test1(1, 2, c=3)
test1(a=1, b=2, c=3)

print("method")
a = A()
a.test1(1, 2, 3)
a.test1(1, b=2, c=3)

On running this code, you should see

function
test1 called with args_cnt=2 & kwargs_cnt=1
test1 called with args_cnt=0 & kwargs_cnt=3

method
test1 called with args_cnt=4 & kwargs_cnt=0
test1 called with args_cnt=2 & kwargs_cnt=2

Its working fine but for methods, its also counting self. so lets fix that!

def counter(is_cls_method: bool = False):
    def outer(f):
        def inner(*args, **kwargs):
            args_cnt = len(args)

            if is_cls_method:   # Check added
               args_cnt -= 1    # Reduced count by 1

            kwargs_cnt = len(kwargs)
            print(f"{f.__name__} called with {args_cnt=} & {kwargs_cnt=}")
            return f(*args, **kwargs)
        return inner
    return outer

This is a simple if clause, but new developer did something else which was interesting use of boolean.

I am showing only the changed code...

   args_cnt = len(args[is_cls_method:])

Solution is lot better than using if, as bool in python is just int. Original code was bit longer and noticing this small change is not obvious, also code base used by users who are basic python users. And, if you have to guess what a line is doing, i think you should change to make it obvious.

What is your thought on this, Do you use boolean as index?
Do you have any more python patterns like this?

Blog

Python: Interesting Code Patterns

Ninad Mhatre

Instead of doing,

Join Our Newsletter. No Spam, Only the good stuff.

Related