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,
# 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 {}
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.
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?
Disclaimer: All resources provided are partly from the Internet. If there is any infringement of your copyright or other rights and interests, please explain the detailed reasons and provide proof of copyright or rights and interests and then send it to the email: [email protected] We will handle it for you as soon as possible.
Copyright© 2022 湘ICP备2022001581号-3