Here's how you can provide constructive and helpful feedback on your colleagues' coding practices.

The most important aspect to me is understanding the use case for the code. Once you and the developer have a mutual understanding of its purpose, you can provide constructive comments including: performance improvements, planning for maintainability/future enhancements and upgrades, exception handling, documentation, and adherence to best practices for the language/platform being employed. Be prepared to not only suggest improvements, but to provide actual examples that illustrate what you are suggesting (i.e., remember that some aspects may be new to the party you are advising).

I put a lot of emphasis on understanding what business use case is, what was the developers understanding on it, how they approached, and if they can draw up their pseudocode calling out the algorithm they followed to solve that problem. Once I get that then I review the code from the perspective of whether it was the best approach, if there is a better way to do it, optimization principles we can apply to get better. This approach just not only prepares developers to write better code but also puts me as someone who is not just sitting there to find faults but a mentor helping them to become a good programmer.

Understanding the purpose and requirements for the code is crucial. Once both you and the developer are aligned on its purpose, you can offer valuable feedback, focusing on performance enhancements and strategies for maintainability and future-proofing the code.

Understanding the code you are about to review is always important. Sometimes you need to dig really deep to understand what the purpose is of some code blocks you don't grasp from the start. Another thing to consider is: run the code, debug it and see what it really does. Sometimes you can not observe or see problems that arouse unless the code is actually run. This may be a problem with objects linking, variables scope or event typos that are not noticeable right from the start. Also it is important to 'zoom out' and embrace the whole architecture to identify the bottlenecks. Threads, message channels, classes layout and their inheritance, design patterns used - these are all points to consider.

For me, knowing the reason why something was built a certain way would help identify areas of improvements. Not only criticizing, but also commending your colleague. It's also for good practice to cite any sources if the code was "inspired" from others. There's nothing wrong with using best practices, but there needs to be some acknowledgement. Through this, they can learn to appreciate the hard work and perhaps improve their understanding of the source code.

Keep supportability in mind. I always request the team to think about the person who would be supporting the customer and debugging issues. Clear comments and some explanation of the flow and logic around complex orchestration help. Personally I enjoy the humor developers add in the comments

Saying 'your code is bad' is the worst way to give a feedback. First, almost every company has its own docs on how the code should be written, what naming conventions are, which patterns/structures are welcomed and which are frowned upon. Make sure that every member of a team is aware of those docs. This will cut costs on review and save ones nerves. Second, everyone has its own understanding of how the code should be layered, which patterns are to be used. Saying 'the code is bad' is not meaningful in this case: one can't read others thoughts. Last but not least: don't be lazy. It is important not only to criticize but to provide a hint or even a solution to your counterpart if you have one.

A style guide is a valuable tool for these interactions. Waiting until code review to introduce more esoteric syntactic guidelines is usually too late. Empowering your developer in advance can help you both avoid them feeling nagged or nitpicked during code review.

Recommend reviewing code with resiliency in mind. If there are external calls or API calls then think about exception and failure scenario. If we need to incorporate circuit breaker pattern to handle retries. We need the code to be complete and handle those exceptions gracefully. Understanding the boundaries of modules and what fall inside the scope or outside is every important to reduce coupling between disjoint systems. Code is not limited to functional code only, you should also review the test cases especially the unit testing code.

I constantly remind the team (and also myself) to build the code in such a way that in the future, we can all understand it. Whether that be useful variable names (I need to stop using "BOB"), detailed inline comments, or a separate documented high-level process flow, just anything to help future de-bugging purposes. Before giving feedback, I ask the developer to explain their code in layman's terms. Then go through the code and share my best practices.