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.