How Not To Provide Feedback When Doing Code Review

Code review, as any other review process, can often be an attack on someone's ego and incite anger and frustration. Being both on sending and receiving end of such code reviews for years, I have learned a few how-not-to-do-it, or as one might say, anti-patterns of providing code review feedback. Let me know if you agree/disagree with me on the following:

• You are always reviewing Adam's code while Adam is never reviewing yours.
• Your feedback is like "Adam, you should break down part of this method into a private method" instead of "we can probably extract a part of..."
• You are always suggesting, for example "we should do x and y and z", instead of asking interesting questions "can we do x? what if we did y?"

Service API and Exceptions

Too often I see I am using a REST/SOAP API to talk to a third party system that frustrates me because of poor error messaging. Here's an example to illustrate a typical frustration:

Call: city_service.update_residence_address(city_dueler, new_address)
Response: <status>Failed</status><error>Invalid address</error>

But the address just seems right to me. So, I need to know specific reason about why the address is wrong. The error message leaves this critical detail. What happens next is, I look for the log files created by the city_service, conceptually supposed to be a third party hosted service. To do this, I SSH to the server, then cd to the logs folder and get lost in many of the cryptic log files. Then eventually I find a log and if I am lucky, I find something like the following:

InvalidAddressException at:...
...
Street number is not listed..
...

Now, I know I had an issue with the streer number. But I can see this only after I looked into the log of the thirdparty server. In an ideal case, I don't have an access to their server. Even if I have it, I cannot easily understand their logs and stack traces! In most API calls to third party applications I have had this frustration to varying degrees.

Here's a lesson learned:
Next time you expose an API for another app, expose error messages as specific as possible - so that, no details is left buried into your log files.