Code reviews in the 21st Century

Find that Bug!

There's an old adage that goes something like: 'Do not talk about religion or politics'.  Why?  Because these subjects are full of strong opinions but are thin on objective answers.   One person's certainty is another person's skepticism; someone else's common sense just appears as an a prior bias to those who see matters differently.  Sadly,  conversing these controversial subjects can generate more heat than light.   All too often people can get so wound up that they forget that the outcome of their "discussion" has no bearing on their life expectancy, their salary, their chances to win x- factor, getting that dream date, winning the lottery, finding a cure for climate change or whatever it is they regard as important! 

Similarly, in the world of software engineering code reviews can end up in pointless engagements of conflict.  Developers can bicker over silly little things, offend each other and occasionally catch a bug that probably would have being caught in QA anyway - that conflict free zone around the corner!

Now don't get me wrong, there are perfectly valid reasons why you may think code reviews are a good idea for your project:

1. Catching bugs sooner means less cost to your project. You don't have to release a fix patch because it's has been caught in development phase - yippee!
2. Code becomes more maintable.  That crazy 200 line method that Jonny was writing with a hangover has being caughted before it has the chance to make itself at home deep in your code base.
3. Knowledge is spread across your team. There are no longer large blocks of code that only one person knows about.  And we all know, when that one person talks about taking a two month holiday everyone panics!
4. Developers make more of an effort. If a developer knows someone else is going to pass judgement on his work, he's more likely to put that line of javadoc to clarify when an exception will be thrown.

However, it would be naive to think that code reviews don't cause problems.  In fact, they cause so many problems many 21st century projects don't do them.  I think they have a place but there needs to be some thought regarding how and when they are done so that they are beneficial as opposed to a nuisance.   Here are some guidlines...

• 1. Never forget TDD 

Ensure you have tested your code before you asked someone else to look at it.  Catch your own bugs and deal with them before someone else does.

• 2. Automate as much you can 

There are several very good tools for Java such as PMD, Checkstyle, Findbugs etc  What is the point  getting a human to spend time reviewing code when these tools can very quickly identify many things the human would waste time moaning about?  I am going to say that again.   What is the point  getting a human to spend time reviewing code when these tools can very quickly identify many things the human would waste time moaning about? 

When using these tools, it's important to use a common set of rules for them. This ensures your code is at some sort of agreed standard and much of what used to happend in an old fashioned 20th century code review, won't need to happen.  Ideally, these tools should be run on every check in of code by a hook from your version control system.  If code is really bad - it will be prevented from being checked in.  Billy the developer is prevented from checking in the rubbish he wrote (when he had killer migraine) that he is too embarrassed to look at.  You are actually doing him favours, not just your team. 

• 3. Respect Design 

In some of the earlier Java projects I worked on, the reviews happened way too late.  You were reviewing code when the actual design was flawed.  A design pattern was misunderstood, some nasty dependencies were introduced or a developer just went way off on a tangent.  The review would bring up these points.  The proverbial retort was: 'This is a code review not a design a review!' .  A mess inevitably ensued.  To stop these problems we changed things so that anyone asked to review code would also be involved - in some way - in either the design or the design review.  In fact, we got much more bang from the buck from design reviews than code reviews.  Designs were of a much higher quality and those late nasty surprises stopped.

• 4. Agree a style guide (and a dictionary) 

Even with the automated tooling (such as Checkstyle, Findbugs etc), to avoid unnecessary conflict on style, your project should have a style guide. Stick to the industry standard java conventions - where possible.  Try to have a 'dictionary' for all the concepts your project introduces. This means, when code refers to them it's easier to check that the usage and context is correct.

• 5. Get the right tooling

If all your developers are using Eclipse (and happy using it) something like Jupiter makes sense.  You can navigate your way through code, debug code and essentially leverage everything the Eclipse IDE does to make your life looking at code easier when reviewing code.  However, if everyone is not on the same IDE (or the IDE is not making your life easier) consider something like Review Board. 

• 6. Remember every Project is different.

You may have done something in a previous project that worked.  But remember, every project is different.  The other project had a certain architecture (may have been highly concurrent, highly distributed), had a certain culture (everyone may have enjoyed using eclipse) and used certain tools (maven or ant).  Does the new one tick the same boxes?  Remember, different things work for different projects.

• 7. Remember give and take

When reviewing be positive, be meticulous but do not be pedantic.  Will tiny trivial things that get on your nerves make a project fail or cost your company money?  Probably not.  Put things in perspective.  Remember to be open to other ideas and to change your own mind rather than getting hung up changing someone else's.

• 8. Be buddies 

In my experience, what I call 'buddy reviews' (others call 'over the shoulder')  have worked really well.  A buddy review consists of meeting up with another team member informally every day or two and having a quick glance (5 - 10 mins)  at each other's code at your desk or their's.  This approach means:

1. Problems are caught very early
2. You are always up to speed as to what is going on
3. Reviews are always very short because you are only looking at new code since the last catch up
4. Because the setting is informal - there is no nervous tension. They're fun!
5. You can exchange ideas - regularly. 

When Tech Leading, buddy reviewing your team members is a great way of seeing if anyone on the team is running into trouble early rather than late.  You can help people and get an idea of everyone's progress all at the same time.  And because of the regular nature of buddy reviews, they become habitual and actually get done.  Something we can't say for many other 21st century code reviews!

In summary, if your project is going to engage in code reviews, they should be fast, effective and not waste people's time.  As argued in this post, it is really important to think about how they are organised to ensure that does not happen.

'Til the next time - take care of yourselves.

Extending your JPA POJOs

Extensibility is an important characteristic in many architectures.  It is a measure of how easy (or difficult) it is to add or change functionality without impacting existing core system functionality.

Let's take a simple example.  Suppose your company has a core product to track all the users in a sports club.  Within your product architecture, you have a domain model represented by JPA POJOs.  The domain model contains many POJOs including - of course - a User POJO. 

package com.alex.staveley.persistence
/**
 * User entity.  Represents Users in the Sports Club. 
 *
 * Note: The SQL to generate a table for this in MySQL is:
 *
 * CREATE TABLE USER (ID INT NOT NULL auto_increment, NAME varchar(255) NOT NULL, 
 *  PRIMARY KEY (ID)) ENGINE=InnoDB;
 */ 
@Entity
public class User {
    /* Surrogate Key - automatically generated by DB. */ 
    @GeneratedValue(strategy=GenerationType.IDENTITY) 
    @Id
    private int id;
 
    private String name;

    public int getId() {
        return id;
    }
 
    public void setName(String name) {
        this.name=name;
    }
 
    public String getName() {
        return name;
    }
}

Now, some customers like your product but they need some customisations done before they buy it.  For example, one customer wants the attribute birthplace added to the User and wants this persisted.  The logical place for this attribute is - of course - in the User POJO, but no other customer wants this attribute.  So what do you do? Do you make a specific User class just for this customer and then swap it in just for them?  What happens when you change your Product User class then?  What happens if another customer wants another customisation?  Or changes their mind?  Are you sensing things are going to get messy?

Thankfully, one implementation of JPA: Eclipselink helps out here.  The 2.3 release (available since June 2011, latest release being a 2.3.2 maintenance released just recently, 9th December, 2011) includes some very features which work a treat for this type of scenario.  Let's elaborate.  By simply adding the @VirtualAccessmethods Eclipselink annotation to a POJO we signal to Eclipselink that the POJO may have some extra (also known as virtual) attributes.  You don't have to specify any of these extra attributes in code, otherwise they wouldn't be very virtual!  You just have to specify a generic getter and setter to cater for their getting and setting.  You also have to have somewhere to store them in memory, something like a good old hashmap - which of course should be transient because we don't persist the hashmap itself.  Note: They don't have to be stored in a HashMap, it's just a popular choice!

Let's take a look at our revamped User which is now extensible!

@Entity
@VirtualAccessMethods
public class User {
    /* Surrogate Key - automatically generated by DB. */ 
    @GeneratedValue(strategy=GenerationType.IDENTITY) 
    @Id
    private int id;
 
    private String name;
 
    @Transient
    private Map extensions = new HashMap();

    public int getId() {
        return id;
    }
 
    public void setName(String name) {
        this.name=name;
    }
 
    public String getName() {
        return name;
    }
 
    public  T get(String name) {
        return (T) extensions.get(name);
    }
 
    public Object set(String name, Object value) {
        return extensions.put(name, value);
    } 
}

So, is that it?  Well there's a little bit more magic.  You have to tell eclipselink about your additional attributes.  More specifically: what their names and datatypes are. You do this by updating your eclipselink-orm.xml which resides in the same META-INF folder that the persistent.xml is in. 
Now this configuration simply states, the User entity has an additional attribute which in java is "thebirthplace" and it is virtual.  This means it is not explictly defined in the POJO but if we were to debug things, we'd see the attribute having the name 'thebirthplace' in memory.  This configuration also states that the corresponding database column for the attribute is birthplace.  And eclipselink can get and set this attribute using the generic get /set methods.

You wanna test it? Well add the column to your database table.  In MySql this would be:  
        alter table user add column birthplace varchar(64)

Then run this simple test:

So now, we can have one User POJO in our product code which is extensible.  Each customer can have their own attributes added to the User - as they wish.  And of course, each customer is separated from all other customers very easily by just ensuring each customer's extensions resides in a specific eclipslink-orm.xml.  Remember, you are free to name these files as you want and if you don't use the default names you just update the persistence.xml file to state what names you are using.

This approach to extending the User, means that when we want to update the User POJO in our product, we only have to update one (and only one) User POJO.  But, when specific attributes have to be added for specific customer(s), we don't touch the User POJO code.  We simple make the changes to the XML and do not have to recompile anything from the core product. And of course, at any time it is easy to see what the customisations for any customer are by just simply looking at the appropriate eclipselink-orm.file.

Ye Ha. Happy Extending! 

References:

1. http://wiki.eclipse.org/EclipseLink/UserGuide/JPA/Advanced_JPA_Development/Extensible_Entities
2. http://www.eclipse.org/eclipselink/

HTML 5 Placeholder Text

Previously, this blog posted an overview of HTML 5.  Let's have a closer look at one of the more useful HTML5 features: placeholder text.   Placeholder text is  a short hint intended to help the user understand what their data entry should be.  It is not default text.

Placeholder example:

First name:

Previously to achieve this involved some javascript; now it's extremly simple.  Simple add the placeholder attribute to the input element.

Placeholder text has some real benefits.  It means you are conveying more information to the user but not taking up anymore valuable real estate space on the page. Now, it is fair to say that there are some very popular GUIs that don't use placeholder text. For example, the login screen for gmail does not.

gmail login

This begs the question when from a usability perspective when does it make sense to use it and when is it best avoided?  Let's think about the gmail case a bit more and that helps answer the question.

By default, the gmail Username textbox gets focus when you arrive at www.gmail.com. This makes sense. The Username textbox is the smart staring point for this page.  It's the logical place for the user to start their interaction.  Now, when a textbox has focus any placeholder text it has always gets wiped. Otherwise, the user would be seeing a mixture of what they are typing and what the placeholder text was.  That would just be horrible! So, since the username textbox has default focus for gmail.com it makes sense that when you arrive, the username textbox is blank. How about the case when the user types nothing in the username textbox and shifts the focus to something else? Would the arrival of placeholder text add meaning or confuse the user?  I'd argue it would confuse them.  Aside from the sudden flicker being an irration, the user could think they did something to cause the text to be put there.   It will confuse rather than elucidate!   As for the password field, well they generally don't have placeholder text - for obvious reasons. So gmail is being smart here. There is no placeholder text but for logical reasons.

So placeholder text is great but there's the question of when it should be and shouldn't be used should not be avoided.

Now, let's dig a bit further into the usability.  The W3C  spec states that the: "placeholder attribute should not be used as an alternative to a label".   I'd also suggest that the placeholder text should not repeat the label.  What's the point in saying the same thing twice?  There's no point in duplication, but there is nearly always a case for elucidation.  A good example is the firefox searchbar.   This is the text bar found in the top right corner of the Firefox window that can be used to search popular search engines without the user having to access them directly.   A number of different search engines can be selected.  In all cases, there is icon for the engine which acts as a label for the search engine.  There is also placeholder text in the search textbox ajacent to the icon label.  This elucidates what the label icon is identifying.  For example, for Wikipedia we see, the Wikipedia icon and then the placeholder text: Wikipedia (en)

Wikipedia selected 

What's really clever is here is the use the very efficient use of real estate.  The icon is very small and in case it is not clear it means Wikipedia, the placeholder text elucidates the matter.  We see the same if Google is selected.

Google selected

So yes placeholder text is a great feature.  It's easy to use but using it at the right time and smartly requires thought.   It all comes down to the simple question: is the addition of placeholder text going to elucidates matters for the user or not? 

So any browser caveats?  Well not all browsers support the placeholder attribute. For example IE 9 does not.   But thankfully, if the placeholder attribute is not understood, it is ignored. There are no nasty exceptions or error messages.  Just have a look at this webpage in IE 9.  As always, if you want to support fallback use the Modenizer library.  There are several examples available using Javascript /JQuery if you want to have placeholder text in browsers that don't support this simple way of doing it using HTML 5.

References:
1. HTML5 spec: http://dev.w3.org/html5/spec/Overview.html
2. HTML5 placeholder spec: http://dev.w3.org/html5/spec/Overview.html#the-placeholder-attribute
3. Site to test your HTML5 browser support: http://html5test.com/
4. Site to test your browser support for input type attribute support: http://miketaylr.com/code/input-type-attr.html

Doing more with less in JUnit 4

JUnit is a popular testing framework for writing unit tests in Java projects. Every software engineer should always try to consider using less code to achieve objectives and the same is true for test code. This post shows one way to write less test code by increasing code re-use using JUnit 4 annotations.
Ok, so let's set the scene. You have a bunch of tests that are very similar but vary only slightly in context. For example, let's say you have three tests which do pretty much the same thing. They all read in data from a file, and count the number of lines in the file. Essentially you want the one test to run three times, but read
a different file and test for a specific amount of lines each time.

This can be achieved in an elegant way in JUnit 4 using the @RunWith and @Parameters annotations. An example is shown below.

W.R.T. to code above:

1. The RunWith annotation tells JUnit to run with Parameterized runner instead of the default runner.
   
2. The @Parameters annotation is defined by the JUnit class org.junit.runners.Parameterized.
   It is used to define the parameters that will be injected into the test.
   
3. The data() method is annotated with the parameters annotation. It defines a two dimensional array. The first array is: "players10.txt", 10. These parameters will be used the first time the test is run. "players10.txt" is the file name. 10 is the expected number of rows.
   

So hopefully this makes sense. Not only is this a way to cut down on test code, it is also a way to provide extra testing of your code. Every time you write a test all you have to do is ask yourself if it makes sense to parameterize your test.
Enjoy...

Make your JAXB cleaner with the MOXy implementation

The principle advantage of using JAXB when marshalling and demarshalling XML is the programming model. Simply annotate a few POJOs and use the JAXB API's and you can serialise to XML and deserialise from XML very easily. You don't need to worry about the specifics regarding how the XML is marshalled / unmarshalled. Everything is much simpler than alternatives such as DOM and SAX.

Now data in XML files tends to be hierarchial in nature. For example, conside this XML file:

In this case, the person Barok Obama has a car which is a Green Ford Focus. Here, we see the hierarchial characteristics of XML. The Car is under the Person. In a more sophisticated example, a Person could have a Car, which has a Car Radio, which has an Amplifier, which has Transistors etc. But let's stick with our simpler case for the moment. Suppose we want to unmarshall this XML file using JAXB. We want all the person details (firstname, lastname etc.) and the model of the car belonging to the person. We create a Person POJO and a Car POJO and annotate as appropriate.


To unmarshall this we simply do:

This all seems very simple - especially when you consider that the Car entity doesn't even need any annotations! However, the Car only has one attribute and it can seem like overkill to have a POJO class for something we only want one attribute from! Remember this is a simple example, imagine if the hierarchial structure was much deeper. Something like an outer entity containing an entity, which contained another entity which contained even another entity and all we wanted was the outer entity and one attribute from very deepest nested entity. It's essentially the same problem but just even more overkill. We would have to ensure there were POJO class for everything in the hierarchy - even for entities which we wanted nothing from. No-one likes code bloat. So what can we do?

Well the first thing we gotta remember is that JAXB is a specification for which there are many implementations for (e.g. JaxMeAPI, MOXy, Metro). If we were to use the JAXB reference implementation (shipped with the JDK, there is not much we can do). We have to have a Car and Person POJO. However, if we use the MOXy implementation from EclipseLink we can use some of its extensions to help us. More specifically we can use the MOXy @XmlPath extension which is inspired from XPath.

Let's see it in action. Here is the updated Person POJO.

So where's the Car POJO gone? Well it's deleted. We don't need it anymore. Bye bye.
Using the MOXy @XmlPath annotation we do not need the Car POJO. This annotation resides in org.eclipse.persistence.oxm.annotations package and to get that on your classpath is really simple. If you are a maven user just add:

To tell your JDK to use MOXy for the JAXB implementation at runtime you put a file named
jaxb.properties in the same directory as your JAXB POJOs. It contains one line:

To ensure you are using the MOXy implementation just check the JAXB context:

You should see something like:

After that there are no changes. The exact same unmarshalling code can be used.
One reason why I really like this extension is because it means less code. This usually means cleaner code and more maintable code. This becomes even more obvious in more complex scenarios where entities are much more deeper in hiearchial structure than this simple example. It doesn't matter if you are using something like XJC to generate your POJOs you still got code bloat.

Remember JAXB set out to be a cleaner programming model than JAXP alternatives such as SAX and DOM but in scenarios with deep hierachies, the profileration of classes using JAXB doesn't make it a convincingly cleaner. Remember, it would be quite easy to ignore the classes you don't want using DOM and XPath or even just using SAX.

MOXy swings the battle for cleanliness back to JAXB by providing the ability to use XPath expressions for anything in our XML file.

Note: MOXy has just being included as JAXB implementation for WebLogic 12c.

References:
1. MOXy project page
2. Blaise Doughan's blog