A meta post about negative code reviews
Join the DZone community and get the full member experience.Join For Free
a lot of people seems to have problems whenever i post a code review. the general theme of the comments is mostly along the lines of:
- you are an evil person and a cyber bully to actually do those sort of things and humiliate people.
- you have something against the author of the personally, and you set out to avenge them.
- there is no value in doing a negative code review.
- you never do any good reviews, only bad ones.
- you only tells us what is wrong, but not what is right.
there are a bunch of other stuff, but those are the main points.
for point 1 & 2, i have the same answer:
i talk about the code , not the person. i am actually very careful with my phrasing whenever i do this sort of a review. the code or the architecture is wrong, not the person. there is a difference, and a big one.
for point 3:
most of those code reviews are generated because someone asks me to do them. and given some of the responses to the posts, i feel that they serve a very good purpose. here is one such comment. i redacted the project name, because it doesn’t really matter, but the point stands:
i grew up on *** as it was my first layered application, almost 5 years ago and i personally believe that the effort ****** has put into this project during the years is simply amazing. the architecture reflect the most common architectural design patterns and represents almost the concepts expressed by fowler and evans. *** is not a project to teach you how to work with northwind and it is not a project designed exclusively for nhb. it is designed to show how a layered application should be architected in .net and there is also a book wrote around this project.
my professional opinioned, backed by over a decade of practical experience and work in the field tells me that the project in question is actually not a good template for an application. and i feel that by pointing out exactly why, i am doing a service to the community.
look, it is actually quite simple. the major reason that i do those negative code reviews is because i keep seeing the same types of mistakes repeated over and over again at customer sites. and the major reason is that those projects follow best practices as they see it. the problem is that they usually ignore the context of those best practices, so it becomes a horrible mess.
what is worse, there is the issue of the non coding architect, when you have someone that doesn’t actually have responsibility for the output making decisions about how it is going to be built. and those things are actually hard to fight against, precisely because they are considered to be best practices. one of the reasons that i am pointing out the problems in those projects is to serve as a reference point for other people when they need a way to escape the over architecture.
for point 4:
it takes something out of the ordinary to get me to actually post something to the blog. the barrier for a negative code review is how much is annoys me. the barrier for a positive code review is how much it impresses me. it is easier to annoy me to impress me, admittedly, but i quite frequently do good code reviews.
the problem is that most good code bases are actually fairly boring. that is pretty much the definition of a good codebase, of course , so there isn’t really that much to talk about.
for point 5:
that usually come up when i do negative code reviews, “you only show the bad stuff, it doesn’t teach us how to do it right”. well, that is pretty much the point of a negative code review. to show the bad stuff so you would know how to avoid it. there are literally thousands of posts in this blog, and quite a few of them are actually devoted to discussions on how to do it right. i have very little inclination to repeat that advice again in every post.
even a blog post must obey the single responsibility principle.
Published at DZone with permission of Oren Eini, DZone MVB. See the original article here.
Opinions expressed by DZone contributors are their own.
Decoding eBPF Observability: How eBPF Transforms Observability as We Know It
Java Concurrency: Condition
How AI Will Change Agile Project Management