Sunday, July 13, 2014

Cooking an elephant carpaccio

Disclaimer: No elephants or other animals were harmed during the writing of this post.


Make me an elephant

What if I asked you to make me an elephant? Can you tell me how long would it take you? Can you guarantee that under this time estimation you gave me you will be able to create a great elephant, with all the capabilities and characteristics an elephant possesses?


Image taken from here

Tackle this huge story

Ok, so us, software engineers, are not that good at creating elephants. We'll leave this to the lady elephants to cook new elephants. But we aim to be good at creating software.
Tackling a large story may be very puzzling. Both from our (developers) perspective and from the product manager's eyes.


How many times have you started working on such an enormous story and found out that there were a lot of holes in your complete mega-design? How many times have you been mistaken in your estimations? How many times have you found out that large portion of your code is not in use or not what the customer/product described?

It's hard for us to plan every little detail in the system ahead. There are a lot of unknowns down the road, which will be careless of us to presume we know, and will make it difficult for us to give an honest estimation. Also it's not easy for the product as well to define every little detail given all these unknowns.


Elephant carpaccio

Elephant carpaccio is a technique that helps us tackle such a big problem. Instead of trying to create an entire elephant, we can try breaking the manufacturing process to small stories. Each such story must stand on its own - meaning, provide some (some == more than 0) value to our users and be independently testable (meaning - no need to wait for other parts of the story to end in order to test it).

In an elephant carpaccio we'll try to follow a simple pattern:
  1. Create a very thin flow
  2. Thicken this flow by each time adding another layer or sub-feature to it.

Creating the initial flow

In order to find this main flow we better ask ourselves what is the main problem we try to solve. We can then map the main use-case. The one that solves a large portion of the problem. Then we can decide this will be the flow.

It's important to understand that this is ok to state that this flow will not stand on its own for releasing to the market. If we would to design an ATM, our main flow would probably be - withdraw money. But we can't send it to production without making sure the flow is transactional, secured, audited in the bank's books and so on. But creating the really basic flow of withdrawing money, in a very naive way (maybe even without checking if the customer has sufficient funds in his account) will already give an enormous value and a great starting point.

Thickening the flow

After we understand the initial flow we're in a much better position to split the story. In this point we need to ask ourselves some questions to understand the more smaller details by which we can split the story. I'll give example on how we do it in my next post.

Focus, Build, Increase Trust

Sometimes in the work of engineers with product we encounter a lot of trust issues (if you speak Hebrew, take 5 minutes to hear from a former colleague of mine about this in this ignite talk or read her blog-post, in English, instead). Product managers often feel they do not exactly understand where the engineering are in the process and whether things go as they wanted. The engineers are not always aligned with the vision the product managers lead to. It sometimes makes them scatter to the less important areas of the feature, the ones that are a nice-to-have (for example - having an ultra beautiful button is important in an ATM, but less important than having the functionality to withdraw money).

By working with small portions we allow product to define the priorities of the work by having the engineering focused and aligned with what's really important. The engineers, when the carpaccio is done right, know exactly what they should do now. They're (usually) not distracted because they work in a very small units of work. It also allows product to change requirements on the way because the engineering process starts with the certainties and the most important things. We can change the rest of the plan as we continue to progress.
When combining such story-splitting techniques with other agile methodologies like sprints, dailies and so on, we can create a better communication between product and engineering, improve trust and improve the productivity of the team as a whole.

My Rules of Thumb for Carppacioing

  1. Split an epic to stories in such a way that each story would provide value. Any value.
    How you define value you ask? Value is defined as something that you can show to a user and (s)he would actually care and be able to give you feedback about it. 
    (A new button that doesn't do anything yet is value; A new table in the DB is not).
  2. Repeat bullet #1 for each story you created and try to split it some more until you're absolutely certain you can't or that further splitting would just increase the overhead of implementing it too much.
  3. In your sprint planning split each story to very short tasks. These tasks do not need to provide user value. They can and should be very small but not too small (less than 0.5 day is probably too small) in order to not increase overhead.

For those of you who would like to deepen your understanding on the subject I would suggest  Lars Thorup's presentation on the subject which I found very interesting and concise.
Also, in my next post I'll give a more detailed example of the carpaccio method. Stay tuned...


Find me on Twitter: @AviEtzioni


More interesting posts from this blog:

Friday, May 23, 2014

So Long Spring XMLs... (@Configuration class quick start guide)

This post is based on a tech-talk I gave in Outbrain

This time I decided to dedicate the post to something a bit more technical than my usual posts. In this post I will try to show those of you that use XMLs to define their Java Spring application context, how to use a method which I find much more convenient for most cases - the spring @Configuration class.

When Spring just started, the only way to configure the wirings of an application, was to use XMLs which defined the dependencies between different beans. As Spring had continued to develop, 2 more methods were added to configure dependencies - the annotation method and the @Configuration method.


What is this @Configuration class?

You can think of a @Configuration class just like XML definitions, only defined by code. Using code instead of XMLs allows some advantages over XMLs which made me switch to this method:

  1. No typos - You can't have a typo in code. The code just won't compile
  2. Compile time check (fail fast) - With XMLs it's possible to add an argument to a bean's constructor but to forget to inject this argument when defining the bean in the XML. Again, this can't happen with code. The code just won't compile
  3. IDE features come for free - Using code allows you to find usages of the bean's constructor to find out easily the contexts that use it; It allows you to jump back and forth between beans definitions and basically everything you can do with code, you get for free.
  4. Feature flags - In Outbrain we use feature-flags a lot. Due to the continuous-deployment culture of the company, a code that is pushed to the trunk can find itself in production in a matter of minutes. Sometimes, when developing features, we use feature flags to enable/disable certain features. This is pretty easy to do by defining 2 different implementations to the same interface and decide which one to load according to the flag. When using XMLs we had to use the alias feature which makes it not intuitive enough to create feature-flags. With @Configuration, we can create a simple if clause for choosing the right implementation.

Our example case

So, let's start with a simple example of a Spring XML, and migrate it to Spring @Configuration class:

<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd">
    
  <import resource="another-application-context.xml"/>

  <bean id="someBean" class="avi.etzioni.spring.configuration.SomeClassImpl">
    <constructor-arg value="${some.interesting.property}" />
  </bean>
  
  <bean id="anotherBean" class="avi.etzioni.spring.configuration.AnotherClassImpl">
    <constructor-arg ref="someBean"/>
    <constructor-arg ref="beanFromSomewhereElse"/>
  </bean>
</beans>


Step 1: Migrate <beans> to @Configuration

In XMLs the highest tag in the hierarchy is <beans>. This tag will be replaced with a class, annotated with @Configuration

@Configuration
public class ByeXmlApplicationContext {

}


Step 2: Create a method for each Bean

Each <bean> tag in the XML will be replaced with a method that's annotated with @Bean annotation. Usually it would be a better practice for the method to return an interface type as follows:

@Configuration
public class ByeXmlApplicationContext {

  @Bean(name = "someBean")
  public SomeClass getSomeClass() {
      return new SomeClassImpl(someInterestingProperty);
  }

  @Bean(name = "anotherBean")
  public AnotherClass getAnotherClass() {
     return new AnotherClassImpl(getSomeClass(), beanFromSomewhereElse);
  }
}
A few things to notice:
  • Each method is defined to return an interface type. In the method body we create the concrete class.
  • The name that's defined in the @Bean annotation is the same as the id that is defined in the XML for the beans.
  • The bean anotherBean is injected with someBean in the XML. In the scenario here, we just call the getSomeClass() method. This doesn't create another bean, this just uses the bean someBean (the same as it was in the XML).
We notice that we're missing the property someInterestingProperty and the bean beanFromSomewhereElse.

Step 3: Import other XMLs or other @Configuration classes

The bean beanFromSomewhereElse comes from a different XML file named another-application-context.xml and which was imported in the original XML. In order to use it, we need to import this XML here as well. To do so, we'll just annotate the class with the annotation @ImportResource as follows:

@ImportResource("another-application-context.xml")
@Configuration
public class ByeXmlApplicationContext {
  . . .
}


That's in fact equivalent to the <import resource=""/> tag in the XML format.
If this bean resides in another @Configuration class you can use a different annotation @Import to import it:


@Import(OtherConfiguration.class)
@Configuration
public class ByeXmlApplicationContext {
 ...
}


In order to complete the picture, here's how you can import a @Configuration class from an XML configuration file:

<context:annotation-config/>
<bean class="some.package.ByeXmlApplicationContext"/>

The <context:annotation-config/> needs to be defined once in the context in order to make spring aware to @Configuration classes


Step 4: Import beans from other XMLs (or @Configuration class, or @Component etc... classes)

In order to use beans that were not defined in this @Configuration class we can either declare a private member annotated with @Autowired and @Qualifier as follows:

  @Autowired
  @Qualifier(value = "beanFromSomewhereElse")
  private final StrangeBean beanFromSomewhereElse;

This member can now be used to construct the bean anotherBean.
Another option is to declare a method argument to getAnotherClass() as follows:


  @Bean(name = "anotherBean")
  public AnotherClass getAnotherClass(@Qualifier (value = "beanFromSomewhereElse")
    final StrangeBean beanFromSomewhereElse) {
     return new AnotherClassImpl(getSomeClass(), beanFromSomewhereElse);
  }
I usually prefer the first method as it is less verbose. But of course, that's just a matter of taste.
Just remember - the beans you import must be loaded to the application context - either by @Import or @ImportResource from this class, or using any other method from anywhere else (XML, @Configuration or annotations).

Step 5: Import properties

So, we still need to import somehow the property someInterestingProperty which was defined in the XML using ${some.interesting.property}. Well, that will be very similar to autowiring a bean, but instead of the @Qualifier annotation, we'll use the @Value annotation:

@Autowired
@Value("${some.interesting.property}")
private final String someInterestingProperty;


You can also use a SpEL (Spring Expression Language) expressions with @Value.


Step 6: Import @Configuration from web.xml

At the final step, we would like to be able to import an entire application-context without using any XMLs. If we have a web app, this can be done by declaring the class in the web.xml as follows:

<servlet>
    <servlet-name>my-dispatcher</servlet-name>
    <servlet-class>org.springframework.web.servlet.DispatcherServlet</servlet-class>
    <init-param>
      <param-name>contextClass</param-name>
      <param-value>
        org.springframework.web.context.support.AnnotationConfigWebApplicationContext
      </param-value>
    </init-param>
    <init-param>
      <param-name>contextConfigLocation</param-name>
      <param-value>some.package.ByeXmlApplicationContext</param-value>
    </init-param>
    <load-on-startup>1</load-on-startup>
</servlet>



Summary

As you can see - spring @Configuration classes can be a powerful tool in defining your application context. But with great power comes great responsibility. Code is much easier to abuse than XMLs. It's easy to make complexed @Configuration classes. Try to think of the @Configuration class as a more flexible XMLs and behave them as if they were XMLs:
  • Split to different @Configuration classes and don't put all of your beans in one class
  • Give meaningful names and even decide on a naming convention
  • Avoid any logic inside the @Configuration classes. Aside maybe for things like feature-flags.

Of course, I just gave here the basics. The internet is full of resources about using @Configuration classes. And of course, you are more than welcome to contact me for any further help. I'll do my best to assist.



Find me on Twitter: @AviEtzioni



More interesting posts from this blog:

Friday, April 11, 2014

My Reversim Summit Experience

Reversim is an Israeli podcast that speaks about technology and the software world. In 2013 they organized the first Reversim summit - an event dedicated to sharing knowledge between tech companies. After last year's success, in February 2014 a second summit was held. The tickets were over faster than the Led-Zeppelin or the israeli Kaveret reunions (I guess that the free-entrance made it easy for people to just register and save their spot quickly, but I think that was not the main reason).


This year I got the opportunity to present my code-review talk, which I gave in Outbrain a few month ago, in front of people from other companies. For those of you who missed it, here it is:



It was very interesting to speak with people after the talk and hear their opinions about code-review, the challenges they faced and the tips they looked for.

Aside from that, there were so many great lectures and things to enjoy in the summit. For me, the most inspiring lecture was Iris Shoor's lecture that talked about how she converted her dev team to marketers. Her last year's presentation inspired me when I worked on my interview exercise for Outbrain, so I knew that I want to hear this year's talk. 
Another great talk was Oren Ellenbogen's talk: Engineering your culture- how to keep your engineers happy. Oren analyzed the qualities engineers look for in a work place and the importance of hiring the people that fit the qualities the company can offer.

Of course, I still need to catch up all the talks I couldn't attend. I believe I'm about to find a lot more interesting talks.

Aside for the regular talks, I really enjoyed the open-space about "Why do we work so hard and move so slowly?". Probably a question that each developer has asked himself/herself sometime. It was very interesting to hear the different opinions and observations to this question. For example, a lot of people talked about all the "non-working" stuff they have to do - answering emails, attend meetings, fix bugs and so on. One of the conclusions that the group reached to was that these things are also work, they also help us progress. A lot of engineers feel that progress == writing code, and not consider other valuable tasks as work which makes them feel they move too slowly.
Also we talked about the importance of retrospecting and measuring your work and other stuff like technical-debt, methodologies and so on.

Also, let us not forget a lot of other fun activities like the ignite talks in which people tried to pass on an idea in 5 minutes, the band (with the Pizza & Beer) and the (in)famous "Hall of Shame" where people exposed their most glorified failures in their career.

So, bottom-line - I really enjoyed this summit, both as a speaker and as a listener. I would recommend other colleagues to catch the videos on youtube. I'm already waiting for next year!


Find me on Twitter: @AviEtzioni



More interesting posts from this blog:

Saturday, February 8, 2014

The Best Programming Language


This post is not about Java vs .Net or Python vs Ruby or any other programming language in particular. This post is about the programming language that is common to all of them. We call these technologies languages but in fact all of them are written in English (well aside for a few). But unlike English article or blog post, programs are often very hard to read. In this post I want to share with you how I try to keep my code readable and maintainable.


Inflation of comments


When I started to program professionally, more than ten years ago, I felt it was pretty hard for me to get into others' code. I was having a hard time reading it. I wanted to make sure my code wouldn't look like that. So, as we often do, I made sure my code was filled with tons of comments. Let me demonstrate with an example (code taken from my refactoring hands-on post, comments added for this post):


That's not the whole class but it pretty much reflects the idea. Now, take a second and try to figure out what this code does more or less.

I bet you didn't read the comments. And if you did, that's only because I have been writing about comments from the beginning of this section. Most of the times we are blind to comments, we ignore them. Especially if there are many of them and we're already used to them stating the obvious. Do I really need a comment stating that this is THE item Dao?



All of the comments in this code are pretty much the same, stating the obvious. This is not readable code, this is cluttered code that needs to be maintained twice - once for the code itself and once for the comments. Problem is - comments tend to rot. We don't have to maintain them, the computer doesn't care what they say, the computer only cares about the code. So, we tend to neglect them. I bet most of you didn't even find the copy-paste mistake hidden in the comments.



Deflation of comments


After I realized that I was inflating comments, I started to deflate them. I wanted my code to feel that when there's a comment, it must be read, it's important. I eliminated all of, what I call, "The Duh" comments.



I only added comments when I really needed to explain what I was doing. For example, what I like to call "The eeeefff if" (because it's such a code-smell):


No way you can understand what this if-clause checks on the first read. You'll have to read it at least twice, and usually 3-4-5 times, to be certain you understand the code. In this kind of scenarios I had usually put a comment that stated what this clause was supposed to do: "Checks if the document has a title or if we can extract a title" (for the sake of the example - let's assume we can extract a title from video/image). Ok, that's a bit clearer now, but we need to consider two things:
  1. People will still, most likely, try to read the code before the comment (assuming they won't ignore it completely).
  2. Even after reading the comment, it is still not clear why we are so certain we can extract a title. The if is complicated, and we can't be sure the comment is aligned with the code and hasn't rotten yet (at least without checking the source-control history).
We'll get back to this example later.


/* No comment */


At the last stage (so far...) in my journey for writing readable code I realized, with the help of Uncle Bob in his book Clean Code, that I should make my code readable without comments. This may sound extreme at first, "A code that is readable without any comments?", well... yes. Aside for some very specific cases, you should aspire to avoid comments in your code and just write code that is readable on its own.



Writing code in English


If you manage to write code with each line looking like a sentence in English, each method is a paragraph telling about a very specific subject of the big picture, and each class tells only one aspect of the story - you will not need almost any comments, and your code will be readable. And also as a beneficial side effect, well-designed and maintainable. Uncle Bob compares this type of code to a newspaper where you have articles (classes) with headers (high-level method abstraction), sub-headers and text which both represent different levels of implementation details.

The two main ways in my opinion to achieve such code are:

  1. Descriptive naming
  2. English-like sentences

Descriptive naming


There's just so much to say about descriptive naming which makes it a topic for another post. I'll just give a few pointers for good naming:

  • Give clear descriptive names which can be used in English-like sentences (will be described in the next paragraph)
  • Avoid using abbreviations
  • Stick to conventions
  • Avoid making the reader scroll up/down the page to figure out what a variable/method/class is meant for. With good naming, usually no scrolling will be needed.
Giving names to things is amongst the programmer's most difficult tasks. I suggest that when you write the code, start with any name (even foo for a method), and improve it as you go along (but before you commit or ask for review).


English like sentences


Try to maintain the reading flow of your code-lines. Make them look like English sentences. For example:



You can read the if sentence like an English sentence "if store has client - currentClient". That is far more readable than doing something like:




The same goes for the ugly "if" we saw before. We can extract it to a private method in the Document class that simplifies the question in a form of an English sentence "if doc has title":

The new if form

The Document object will encapsulate the answer to the question by calling two private methods that each simplifies another aspect of the question: 


Each part of the question is simplified in its own method


This way completely abstracts for the readers the question "has title?" and its two sub-questions - "already has a title?" or "can a title be extracted?" The proximity of the methods in the code also counts, but I won't get into that in this post.



Is it ever ok to use comments?


Of course! Comments are needed for java docs, they are needed to explain a nasty hack, explain why you preferred one algorithm over the other (when it's not obvious) or add a TODO comment (but if it stays there for more than a few days - it's also a code-smell). Also, each rule has its exception and sometimes comments may be needed to explain the code. But try to make it the exception, and not the norm, to use comments to explain yourself.


Further Reading


This post was longer than most of my posts and I haven't even started to scratch the tip of the iceberg on the subject. I invite you to read Uncle Bob's Clean Code in order to improve your code-writing skills. Also, don't hesitate to contact me (via Twitter, Linked-in or just leave a comment here) for more help on the subject.

I also invite you to send me code examples with explanation comments (even if we don't know each other yet) and we'll try to figure out together how to remove them and create a clean, readable code instead.



Find me on Twitter: @AviEtzioni



More interesting posts from this blog:

Monday, January 6, 2014

Hands On: Effective Refactoring of Legacy Code

In my previous post I talked about refactoring. At the end of it I promised to give a hands-on tutorial on refactoring.

The last two weeks at Outbrain were great fun when we had our "Quality Time". In this quality time we invested two whole weeks in reducing our technical debt. As part of this quality time, people gave tech-talks about quality. As a fan of code-quality and continuous improvement I gave a talk about my team's first retrospective and another talk which was a hands-on tutorial on refactoring legacy code. Here it is, I hope you'll enjoy it. 




This talk is based on Testing and Refactoring Legacy Code which I really liked so I decided to translate it to Hebrew and change it to include common smells in Outbrain's legacy code. 


Find me on Twitter: @AviEtzioni



More interesting posts from this blog:

Friday, December 27, 2013

Refactoring - Invest in your future

Legacy code


Legacy code - This phrase gives most of us the shivers. We imagine legacy code as a messy, unreadable, complicated, written-with-old-technology-and-conventions code. We embrace the dreadful phrase "if it works - don't touch it". We try to avoid as much as possible touching this horrible thing called Legacy code.

Smelly code: Picture taken from here

What exactly is legacy code?


I tried to get an answer to this question. Is it a code written using old technology? A code written by somebody who doesn't work for the company anymore? A code that is very fragile or complicated?

Well, I guess all of the above are correct. But I believe code becomes legacy code the moment it is too intimidating to be changed. No matter what the reason is. If you wrote a new feature today, which took you a lot of mental energy and time, and tomorrow you'll be afraid to change it, then I've got news for you - you have a new-born-healthy-two-days-old legacy code. Congratulations!!!


Refactoring


As time passes, our code becomes harder and harder to understand and maintain. The developers who touch the code come-and go, functionality evolves which causes much more flows that serve an ever-growing demand of use-cases to appear, technologies change but each of them leaves its signature on the code-base. 

Refactoring is our way, as engineers, to clean up the mess.  Some people think that refactoring is a matter of aesthetics. While this may be true, aesthetics is merely a mean to an end - making the code more easily-maintainable and more readable to others. And when I say others, I also mean - the future you. You are the one who most likely will read this code in the foreseeable future, and I bet you prefer not to struggle with it.


Invest in your future


A lot of engineers fail to realize that refactoring is an investment. You invest in order to save much more later. Every minute that is spent trying to understand the code, is a minute wasted. If the code is read by different people, in different times, and each of them struggles to understand it because it's complicated - that's a lot of time wasted. If only one of them would break the habbit and invest in a one time effort for a better future - a lot of development time would be spared and could be invested in new features.



The fear factor


And still, people, mostly, but certainly not exclusively, juniors, afraid of refactoring. And I have to admit, there are good reasons to be afraid. How do you start to read this code? It's so complicated and full of variables and branches and try-catches and too many lines of-code and code-duplications.

It's also very intimidating to refactor when the person who wrote most parts of the code does not work with you anymore. Or maybe it was you who wrote it but you just can't remember why the hell you did it this way, but you're sure there was a reason.


Eliminating the fear factor

Luckily we have so many tools in our disposal that, when you think about it, completely reduce the amount of fear one should have when refactoring.

You can always revert back to the way things were before the change using the source control.

Tests are key for an efficient refactor processYou need to have tests that test the code. If you don't have tests (or don't have enough) - write tests before starting to refactor. Not only will it improve your confidence that you don't break anything while refactoring, but it will also help you to better understand the code. 

Try to find ways to control and monitor your changes. For example, in Outbrain, we use feature flags in order to control new features. When we want to implement a new change, we create a flag that instructs the service on whether to use the new feature or not. This can be easily used for refactoring. And of course, it helps reverting in no-time. Just turn-off the feature and you're back to the way you were before the change. We also monitor a lot of metrics in every part of the flow. Usually, I can easily monitor the behaviour of a flow and see if anything changes. Even if you don't have advanced monitoring tools, you could utilize the log for monitoring the changes. Find your way to control and monitor the changes, this will greatly ease your fear.

Suspect comments. Comments are deceiving. They lead you to think that the code does something which it may or may not do. Computers don't understand comments, they just read the code. You should trust the code. If there are comments, use them as a guidance, but be very suspicious about them. Comments rot much faster than code. Also try to write code that is readable without comments when you refactor.

Take baby steps. Don't ever ever ever try to make too many changes at once. Always do one simple change and try to test it. You should allow yourself to be certain what was the exact change that caused problems. Don't make a project out of it. Refactoring is not a "all or nothing" deal. Every change you make, even the tiniest one, is blessed.

Boy scouts rule. Aside from taking baby steps, try to embrace the boy scouts rule - always leave the place cleaner than it was when you got there. While you work, do minor refactorings all the time. It's pretty easy and doesn't take a lot of time or effort. If everybody does that, the accumulating affect will create a much more robust code-base.

Invest only where necessary. If you see an ugly code which is very rarely touched, don't refactor it (aside from, maybe, very minor refactorings). It's not worth the effort. Invest in places to which you come back again and again. 

Use refactoring to learn. You already need to change and extend a piece of code. While refactoring you make the code readable for yourself. You start with minor refactoring, and bit-by-bit you unravel the code and learn how to extend it as you go.


What's the worst that could happen?


When I started working at Outbrain, I had an on-boarding with Ori Lahav, Co-Founder of Outbrain. Ori told us to embrace the phrase "What's the worst that could happen?" and told us to not let fears limit our progress.

When you refactor you should think exactly that - what's the worst that could happen? If you're not touching an extremely sensitive component, then it's no biggy. If you do refactor a sensitive component, there are probably, and hopefully, enough tests around it, and monitoring, and backups, and failover mechanisms and so on. And, of course, you'll take baby-steps and test and monitor the change carefully. So go ahead and refactor. What's the worst that could happen??

What's the worst that could happen?

My next post will include a video tutorial that shows how to refactor a piece of legacy-code using TDD. Hope you already can't wait!


Find me on Twitter: @AviEtzioni



More interesting posts from this blog:

Friday, December 20, 2013

Code Review - Just Do It! (Part 3 - What's Your Excuse?)

In my last 2 posts I talked about code-review as a great feedback tool, and after that I tried to give some numbers and reasons as to why you should do it. If you got up to this post, you probably a bit more convinced about code-review. And yet, people tend to give me (and themselves) a lot of excuses as to why they shouldn't code-review. 

This final post about code-review, will be dedicated to the various excuses and their refutation.


It takes too much time


The first excuse is the one I most strongly refuse to accept. Saying that code-review takes too much time implicitly implies that it's a waste of time. In my opinion, a software-engineer's work is to provide quality code in order to allow future maintenance of the software. A lot of other things take time - like talking about and writing designs, like monitoring our app and let us not forget automatic tests that require us a lot of time as well.
Code review is not a waste of time - it's an investment of time that will later save us much more time.


I'm not familiar with the code


I relate to this excuse more as more of a "reason" or "concern". That's true, it's harder to review code that you're not familiar with. But it's much easier than you'd might expect. It's so easy to spot code-smells - too many branching in a method, long methods/classes that do much more than they should, bad-namings, irrelevant and misleading comments, etc...

Let's look at the following code:


Almost every java developer would see that concatenating over and over will cause the heap to be filled with unused objects and the GC to work harder (let's assume no compiler-optimization). These examples (maybe a bit less trivial) happen a lot, and they need you to spot them.

Another great insight you can provide would be in a form of a question. "Did you run tests?", "How does this change affect other components?" and so on. These insights are very valuable. When we code, we don't always remember the full-picture around our feature, and it's great to have someone reminding us (before we discover the issues the hard way).


It's too subjective


That's true. Code-reviews are subjective. And yet, researches show an average of 82% of agreement amongst reviewers. This means that in 82% of the times, it's not that subjective. Also, you could always add more reviewers to help you decide when there is a difference of opinions. And lastly, you don't have to agree, but at least hear what others have to say. It might be useful.


It's just a minor change


Oh, what a devious excuse is this one. No matter if we're totally noobs in software-engineering, or we're experts with 20 years of experience, we fall for this one time after time. We just wanted to change a little string, maybe even a log line and BOOM (!!!), an exception is thrown. Let's take this "minor" example:





Won't it be sad when the NullPointerException will be thrown?

In our line of work, even this minor change can cause problems. Problems that could easily be avoided during a code-review.




JUST DO IT

If you've come this far, I hope I managed to convince you that code-review is something you should embrace in your organization. The best way to start is just doing it. Stop making excuses as to why it won't work and just start. The rest will follow.



Hope you enjoyed and learned from this 3-posts-long topic. In my next post I'll talk about refactoring of legacy code. Stay tuned ;)


Find me on Twitter: @AviEtzioni


More interesting posts from this blog: