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 withdict
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
matchinstead 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?
Top comments (0)