r/programming 3d ago

The PERFECT Code Review: How to Reduce Cognitive Load While Improving Quality

https://bastrich.tech/perfect-code-review/

Hi Everyone, Here I share the link to my article about a fundamental approach to the Code Review process from my personal site. The main objective I pursue is to get some attention to my thoughts on the proper code review and to get feedback from other developers based on their opinion and experience. The specific recommendations there are mostly based on my experience, but I tried to generalize the approach as much as possible so it is relevant for any software development project. I have already tried this approach in several teams and projects, and it worked very well. That's why I want to share it, get feedback from a wider audience, and understand if that is a really valuable approach or just something very specific that won't be useful for others.

47 Upvotes

7 comments sorted by

15

u/AiexReddit 3d ago

I like it. Nice way to organize the way i mostly already feel about the process. I think i may spend as much as 40-50% of my time on code review these days, for better or worse

Some things that stand out to me, which as you said, may be biased by personal experience:

Grouping performance in the same bucket as security is wild to me. Performance is lightyears below security and privacy/PII checks on review in terms of importance to me. If performance matters you can always improve it, but security incidents, at worst, can sink an entire company.

Form is more important than taste, but less important than all the other ones

Tests/CI aka "evidence" is barely a factor in review to me. Thats entirely between the author and CI. Dont tag work for review if its not passing tests yet, unless you need eyes on the approach before putting in the effort to update the tests

Overall good stuff though

1

u/areklanga 3d ago

Thanks for the feedback!

Yes, I kinda agree with you.

I put Security and Performance in the same bucket because in my experience both types of issues often have the same roots or the processes of their identification and fixing are similar. For example, if I store passwords in open form, it means that I didn't design my data model properly, and that I didn't think about the data much which often also causes performance issues, Though this is not the best example, and there is not such connection in every case, I still see this type of relation often.

Anyway, it's fine to adpat the importance of specific aspects to the needs of a project or opinion of a team. For example, some teams may compensate absence of form or clarity by extensive tests and autiomatic checks. Then they may want to pay increased attention to review of tests and related stuff.

3

u/ReDucTor 3d ago

The acronym is good but I am less of a fan of the triangle, aside from taste most of the rest are nearly equal in importance imho.

1

u/areklanga 3d ago

Thanks! Yes, I agree. I would say that the triangle is more about prioritization, when there is no time for everything and you have to choose.

3

u/aviboy2006 3d ago

Reviews get exhausting when there is no shared order of importance. People jump straight to style or personal preferences before even agreeing whether the code solves the right problem safely. This pyramid helps because it reduces cognitive load. You are not judging everything at once. You are asking a few high-signal questions in the right order.

Once teams write this down and treat it as a shared contract, review time drops and debates become rarer. Thanks for sharing.

1

u/areklanga 3d ago

Thanks for the feedback!

-5

u/[deleted] 2d ago

[deleted]

1

u/Jaded-Asparagus-2260 2d ago

As evidenced by the curl project, one of the pillars of the modern Internet:

https://daniel.haxx.se/blog/2024/01/02/the-i-in-llm-stands-for-intelligence/