Conf42 Enterprise Software 2021 - Online

The Perfect Code Review

Video size:

Abstract

Code Reviews shouldn’t hurt. They shouldn’t be depressing or dissapointing. Instead they should show you the best way your code can look and leave you energized to do it yourself.

Teaching one another and looking for issues along the way. Whenever regular Pair Programming is not feasible for your project, this talk gives many tricks you can use the next day.

Summary

  • Victorenta is deeply in love with refactoring, sequential design and unit testing. He founded a community in which to share his passion about this. Feel free to join us if you want. The easiest way to reach him out is on Twitter.
  • The only way we ever learn to program is by locking ourselves up in our room and begging our head on the keyboard. This is how education builds us, basically, we learn programming alone. The best code review I ever saw that I ever took part in was amazing.
  • The goal of a code review is to understand. You shouldn't start off reviewing code with this attitude of finding some terrible bug in there. One typical idea is to include those topics in a team design meeting.
  • Be kind and be humble on both sides. Don't take it personal, never take it. Always focus on learning. More important to keep a team straight than to have the best coding style. Do find me on the social media.

Transcript

This transcript was autogenerated. To make changes, submit a PR.
Hello. Hello and welcome to my tab, the best code reviews before we even start, I want to tell you that there is also an entry on my blog discussing these ideas. So if you want to share your thoughts, you can find me not only on social media, but you can also comment on that blog. Just a few words about me before we begin. My name is Victorenta. I'm a Java champion and I am deeply in love with refactoring, sequential design and unit testing. I even founded a community in which to have a place where to share my passion about this. It's called Buckley Software craftsmanship Community and it's currently one of the biggest developer communities in my country. Feel free to join us if you want. We are on meetup and I'm trying to engage every month with people in the community to discuss different topics regarding these things that I really love. I have a website. On my website you can find a collection of my best talks, the blog and my trainings because that's what I do for a living. I am an independent trainer, teaching of course, Java, the Java standard, technical stuff like spring hibernate, functional programming and Java performance, plus thumbprints that don't have anything to do with Java in particular, but I do it like design pattern, spring or the new index. Exactly the things that I love. I have the great pleasure to also be discussing with hundreds and hundreds of teams throughout the world. In case you want to contact me for joining, here's my website and you should know that I'm also on social media. The easiest way to reach me out is on Twitter. Right? So how do we start coding? How do one learn to program? Don't really learn to program by talking to others. By chatting to others? No, the only way we ever learn to program is by locking ourselves up in our room and begging our head on the keyboard for hours and hours, many painful nights. This is the default way that people start learning. But you see, then you come to work and the first thing you realize is that you have a team. And then after you managed to make it work, ehoo, you've done your job, you've done your task, then your lead or someone from your team called you for a code review. The first time you ever hear this, you're like confused. What do you mean? What does it mean to have a code review? Mean like someone else will look on my code. But why? You have to realize that we as a professional have this tendency of being not the most social beings that you can ever find, because this is how education pushes. This is how education builds us, basically, like, we learn programming alone. So then the first time you realize that you want material and you should work together, you're like, what? But then you go to the code review, and I don't want to go into details about the first code review that you will ever get, but let's push time forward. For four years, you are already coding and you have a spec. You receive the event, you have received a spec, right? The first thing you do, I hope you at least read it, 20% of it at least. Then you start coding, right? How do you read? How do you really do your job? You copy it from there, copy from there, from stack, overflow, google a bit, copy some code that you've seen there. Then maybe after several hours after, you have this feeling that you've seen that method doing pretty much what you want right now. You've seen that method several days ago, but where was that? You can't find it. And what do you do? Yeah, you end up to be pasting a bit of code and so on, and the feeling that you don't really remember what's there, but you make it work maybe several hours after you find the method that you were looking for. But then it's harder to adjust what you do. You leave it like that. What the heck? It's safer to leave it like that. And this keeps going for several hours, maybe days, and at the end, and in the end, you think you're done, but deep down inside, you know that it's not quite the best work you could ever do, right? And then someone else on the team, let's imagine your lead, will call you for a code reviews. You know what will follow, you anticipate what will come next. So then from now on, let me switch to the best code review I ever saw that I ever took part of. It's amazing. One of my old friends, Aline, with his name, with one of the older, more than 15 years of experience, one of the most experienced developers in his company, and he was conducting such code reviews. Like I will describe for you now, when Ali called you for a code review, he will invite you to sit here to have a seat. Arm chair, a very comfortable chair he kept right next to him. He was not sitting on that. He was sitting on some chair, like, not very comfy. But this is the chair for the person who was coming to for a code review. And then he offered you some tea if you didn't fancy. Probably he will offer you some chocolate or whatever he knows that you love. When you were sipping your tea, he jumps right straight to the code and it's clear that he already was looking at that code for a while before calling you. And he starts by suggesting different ideas. Different. How about doing that little improvement? And you say, yeah, that's a good idea. And then immediately after Alim does it, typing it in front of you, he was Alphabet to mention. He was one of the fastest coders I ever had the honor to work with and very, very bright, with great vision before it could be implemented, and so on and so on. Different improvements, you would agree, because they were typically good ideas. You just didn't follow that in that perspective. But then at some point he would suggest something that, what about this other change? Then you say, I'm not really convinced. Right after Ali makes sure that you really understood what he was saying with the benefits of his suggestion, when he's absolutely sure that you got what he meant, then if you still don't agree, he said, okay, just lie to him so he wouldn't push hard. He shouldnt try to convince you about his opinions, if I may say so, the experience was a very interesting one. And this kept going for several minutes, maybe 15 minutes, maybe even half an hour, up to a point in which the code was almost ready to be pushed. Almost. The tests weren't yet fixed. Yeah, he commented out several parts of the code, but the main idea was clear for you, was crystal clear for you. And along the way, along those little refactoring steps, as he cleaned up the code here, it became apparent that refactoring, that bigger refactoring, that deeper refactoring that you thought you saw right before finishing your task, there was this idea that popped in your head. What about that? You couldn't really see the big picture. But now following those little refactorings of Aline, it's crystal clear. And you see it right there in front of you. That's the change that you need to do. You need to break that path into and split that in separate. It's now obvious. So you are like, okay, okay, Aleen, go do that and we'll finish. Arlene tries to start sketching that thing that you suggest and sees that you are now full of that joy of discovery. Yeah, that's what I was about to discover several hours ago. And then. Exactly then, exactly when you were begging to Aline, please commit, please commit. I will continue from here. Exactly then. Exactly then Aleen reverts everything, reverse everything and tells you, no, you do it. Can you imagine the energy that with which you leave that little discussion of yours. Yeah, you're like, come on, I've seen it done in front of my eyes. I can do this. And you do it from scratch again yourself this time. Okay. You don't remember all the little steps Alin performed. Okay. Okay. You miss some of the little steps, but the big picture is there and you end up doing that bigger refactoring that you wanted to do. It takes you several hours. Yeah, there are best to adjust. There are a little bit more complexity in there. But yeah, you managed to make it yourself. Now let's put all this, first of all, I want to .1 thing. Do you realize what Aleen just did when he saw that code review was too complex to solve in little lines of comments or an email? What he did called you up for a little ad hoc per programming session because nothing best a pair programming. However, how would you do that in this time? Imagine that your code reviews, reviews keeps, doesn't start his camera. What happens is that you stare for those minutes to his profile image, which is this. What do you feel as the reviewee of the guy who was the author of the code when you are talking to a dog because the reviewer doesn't want to start up his camera. That's not fair, right? So the first thing you do, the first thing you do, you start up video for any complex code review that you have to perform. You use nonverbal gestures, you use mimic, because this way you can make things softer. You can convey your hesitation about some topic. You can be very firm on other topics. You can interact you as a reviews can see the reactions of your reviews. You can see his hesitation. So any code review, any complex code review that you have to perform these days, please, please start up your video call and be very attentive to the face of the one who. Very attentive. Now, what is a bit, there's one interesting thing here. What is the goal of a code review? What is the attitude with which you should start code review? What are you after? You the code reviewer. Let's assume you shouldn't be looking for errors. You shouldn't start off reviewing code with this attitude of, oh, I know, I will find some terrible bug in there. That's not the point. That's not the point. What you should strive instead is to understand, because the code review is actually a best of readability, of understandability, how easy it is for me to understand that. That's the ultimate test. Someone else who looks at that code and has to understand it. So you should strive on understanding that finding bugs or holes in the implementation is just a side effect. Your focus as a code reviews is understanding. You're basically learning the solution. And also for the reviewee, your goal is also to learn something from this review. Now, there will be some recurring topics in those code reviews, some things that you find yourself telling over and over again. What would you do with them? A bit strange to repeat yourself every time. But one typical idea, one good idea, is to include those in what I call a team design meeting. Now I see, unfortunately very few teams doing on a regular basis such meetings. What is a team design meeting? It's not retrospective in its perfect sense. It's not a project grooming session, although it might be somehow related. No, it's a meeting focused just on design, on coding styles and practices and ideas. And that's a perfect place to raise up the topics that were discovered or debated during the past several code code reviews. So every time you see the reviewer that there is a topic which the others in the team might benefit, what I will do is I will extract from that code, from that code sample. That's a thing that makes it interesting, that is really worthy of being shared. And maybe doing it during a brown bag session, or even doing these official team design meetings of 30 minutes 1 hour every two weeks, I would share that code sample, that distilled code sample with others, maybe from other teams including, and share my findings and debate them. Debate it with. That's a place to discuss the recurring topic so that everyone can have the chance to say his opinion and share thoughts about that. Unfortunately, however, there are many code reviews. You do find some terrible issues like comparing string with equal with equal equal. Or comparing long the rapper class, right? Long big big L L comparing them with equal equal, which is terrible. What the heck? What's happening now? You have to realize one thing. You was a code reviewer. You are by definition frustrated. You are from start supposed to understand the thoughts of another designer to reverse engineer their train of thought. You are by default in a passive aggressive stance. You are by default in a stance of being overwhelmed in a sense, or at least, I don't know, annoyed by the code that you have to understand. It's not your code. It's not your thoughts in there. So when you are in this stance and you find a terrible issue, many people just explode. What's wrong with you? Who hired you? Don't ever do that. If something that you never expected him to do that, please grant him a second chance. Call him, let him look at the code maybe help him a bit and let him discover the issue. Don't say much, give him a second shot. Now there are some cases in which the author because it's new to the team or is just maybe inexperienced. In that case, you should not hesitate to switch to a more guided review style like a more teaching style. And when you see a horror finding like a terrible issue, you should talk to the author and see his reaction. If you didn't expect that, then that's a place where you should insist. Dive more in and see and explain the issues. Oh, and one last thing. Never confuse opinions of fact. Never. You as a reviewer, try to force your reviewee into your mindset, into your coding style. You have different minds. Not a developer writes the identical code. You shouldn't write the identical code. But it's hard really to know what our opinions are. You have to be very experienced to know what are the things which really impede the maintainability of the code, which are the things that you really annoy you. Which are the things which cause bugs or cause issues with understanding the code and focus on those. Don't mind if whether he accepts the constant or not from typically there are so many examples of not very relevant coding guidelines. If you now to summarize a bit, there are some practical tips for some more technical tips for the reviewee for the author. Now of course, work hard, but work hard until you're proud of it, until you are really very proud, until that code becomes worth being added to your cv as a code set. And of course, never forget to run sonar with it. Before you commit code, there are some tools which should check for the trivial scenarios or errors or bugs. Ask for infined code reviews for those more complex because the whole discussion was only about complex features, complex stories that require complex code reviews. For those complex development efforts, ask for in flight reviews, ask for the personnel more senior developer probably to throw an eye on some code that you have right now on your development branch. Let's say in your feature branch. Ask for intermediate code review. Then do micro commits, very short, tiny microcommits, several lines of code, several classes maybe that is. And always, always make sure that you commit large scale refactoring that you've done with automatic refactoring tool like intelligent. It gives you to rename a package that will impact thousands of classes. Maybe commit those separately from the other places that you've manually edited, the boolean conditions, or all those places that can easier that are easier to cause bug because you can keep the attention of your reviewer to what really matters, to the places in which you use your brain to some large scale automatic refactor. I mean, these are very unlikely to break. And also sketch a little diagram, a little supporting diagram. There are several tools you can use for that. There is one which is a whiteboard that you can write on and you can write it collaboratively. You can basically share this to other teams and you can write it together. Or there are UML websites that you can basically describe the UML in a text and then you can generate the UML diagram based on that text document a bit. The more complex things that you submit for review. Now, in general, some code review tips. Be kind and be humble on both sides. You as a reviewer and you as a reviews, never make it or take it. You might face at some point some reviewer which is very aggressive for any reason that might be. Don't take it personal, never take it. Always focus on learning. What have I learned? How did it help me become better? Have a constructive attitude and again focus on learning. There are a lot of distracted opinions and if you have doubts, submit that idea, that discussion to a good design or to a brown box question. Ask the opinion of the other developer. And also something that Zalin did sometimes allow imperfect code to be accept imperfect code. He found more important to keep that developer from on his side, to maintain a positive attitude towards that developer as opposed to imposing his coaching style. Bruce Toby do tolerate imperfect. And of course Aleen had different expectations. If someone very experienced submitted some garbage, he wouldn't scream of course. But if someone less experienced would submit the same thing, he kindly, softly might even actually get to accept some of them. Because it's more important to keep a team straight than to have the best coding style. And of course, keep in mind that petrolming is always better. It's a bi directional communication redirectional learning channel. So always keep whenever you face a complex coding again and do find me on the social media. My social channels are on the website in case you want to reach me. Thanks a lot, have a nice conference.
...

Victor Rentea

Technical Trainer & Consultant

Victor Rentea's LinkedIn account Victor Rentea's twitter account



Join the community!

Learn for free, join the best tech learning community for a price of a pumpkin latte.

Annual
Monthly
Newsletter
$ 0 /mo

Event notifications, weekly newsletter

Delayed access to all content

Immediate access to Keynotes & Panels

Community
$ 8.34 /mo

Immediate access to all content

Courses, quizes & certificates

Community chats

Join the community (7 day free trial)