October 31, 2010

Google Guava v07 examples

We have something called Weekly Technology Workshops at TouK, that is, every Friday at 16:00 somebody has a presentation for everyone willing to come. We present stuff we learn and work on at home, but we also have a bulletin board with topics that people would like to listen about. Last week Maciej Próchniak had a talk about Clojure, this time a few folks asked for an introduction to Google Guava libraries.

Since this was a dead simple task, I was happy to deliver.

WTF is Guava?

It's a set of very simple, basic classes, that you end up writing yourself anyway. Think in terms of Apache commons, just by Google. Just to make your life a little bit easier.

There is an early (v04) presentation and there was a different one (in Polish) at Javarsowia 2010 by Wiktor Gworek.

At the time of writing this, the latest version is v07, it's been mavenized and is available at a public maven repo.

Here's a quick review of a few interesting things. Don't expect anything fancy though, Guava is very BASIC.

@VisibleForTesting


A simple annotation that tells you why a particular property access restriction has been relaxed.

A common trick to use in testing is to relax access restrictions to default for a particular property, so that you can use it in a unit test, which resides in the same package (though in different catalog). Whether you thing it's good or bad, remember to give a hint about that to the developer.

Consider:

public class User {
    private Long id;
    private String firstName;
    private String lastName;
    String login; 
 Why is login package scoped?

public class User {
    private Long id;
    private String firstName;
    private String lastName;
    @VisibleForTesting String login;
Ah, that's why.

Preconditions


Guava has a few preconditions for defensive programming (Design By Contract), but they are not quite as good as what Apache Commons / Spring framework has. One thing interesting is that Guava solution returns the object, so could be inlined. Consider:

Using hand written preconditions:
public User(Long id, String firstName, String lastName, String login) {
        validateParameters(id, firstName, lastName, login);
        this.id = id;
        this.firstName = firstName;
        this.lastName = lastName;
        this.login = login.toLowerCase();
    }

    private void validateParameters(Long id, String firstName, String lastName, String login) {
        if(id == null ) {
            throw new IllegalArgumentException("id cannot be null");
        }

        if(firstName == null || firstName.length() == 0) {
            throw new IllegalArgumentException("firstName cannot be empty");
        }

        if(lastName == null || lastName.length() == 0) {
            throw new IllegalArgumentException("lastName cannot be empty");
        }

        if(login == null || login.length() == 0) {
            throw new IllegalArgumentException("login cannot be empty");
        }
    } 

Using guava preconditions:
public void fullyImplementedGuavaConstructorWouldBe(Long id, String firstName, String lastName, String login) {
        this.id = checkNotNull(id);
        this.firstName = checkNotNull(firstName);
        this.lastName = checkNotNull(lastName);
        this.login = checkNotNull(login);

        checkArgument(firstName.length() > 0);
        checkArgument(lastName.length() > 0);
        checkArgument(login.length() > 0);
    }
(Thanks Yom for noticing that checkNotNull must go before checkArgument, though it makes it a bit unintuitive)

Using spring or apache commons preconditions (the use looks exactly the same for both libraries):
public void springConstructorWouldBe(Long id, String firstName, String lastName, String login) {
        notNull(id); hasText(firstName); hasText(lastName); hasText(login);
        this.id = id;
        this.firstName = firstName;
        this.lastName = lastName;
        this.login = login;
    } 


CharMatcher

For people who hate regexp or just want a simple and good looking object style pattern matching solution.

Examples:

And/or ease of use

        String input = "This invoice has an id of 192/10/10";
        CharMatcher charMatcher = CharMatcher.DIGIT.or(CharMatcher.is('/'));
        String output = charMatcher.retainFrom(input);
 output is: 192/10/10

Negation:
        String input = "DO NOT scream at me!";
        CharMatcher charMatcher = CharMatcher.JAVA_LOWER_CASE.or(CharMatcher.WHITESPACE).negate();
        String output = charMatcher.retainFrom(input);
 output is: DONOT!

Ranges:
        String input = "DO NOT scream at me!";
        CharMatcher charMatcher = CharMatcher.inRange('m', 's').or(CharMatcher.is('a').or(CharMatcher.WHITESPACE));
        String output = charMatcher.retainFrom(input);
output is: sram a m

Joiner / Splitter

As the names suggest, it's string joining/splitting done the right way, although I find the inversion of calls a bit... oh well, it's java.
        String[] fantasyGenres = {"Space Opera", "Horror", "Magic realism", "Religion"};
        String joined = Joiner.on(", ").join(fantasyGenres);
Output: Space Opera, Horror, Magic realism, Religion

You can skip nulls:
        String[] fantasyGenres = {"Space Opera", null, "Horror", "Magic realism", null, "Religion"};
        String joined = Joiner.on(", ").skipNulls().join(fantasyGenres);
Output: Space Opera, Horror, Magic realism, Religion

You can fill nulls:
        String[] fantasyGenres = {"Space Opera", null, "Horror", "Magic realism", null, "Religion"};
        String joined = Joiner.on(", ").useForNull("NULL!!!").join(fantasyGenres);
Output: Space Opera, NULL!!!, Horror, Magic realism, NULL!!!, Religion

You can join maps
        Map<Integer, String> map = newHashMap();
        map.put(1, "Space Opera");
        map.put(2, "Horror");
        map.put(3, "Magic realism");
        String joined = Joiner.on(", ").withKeyValueSeparator(" -> ").join(map);
Output: 1 → Space Opera, 2 → Horror, 3 → Magic realism

Split returns Iterable instead of JDK arrays:
        String input = "Some very stupid data with ids of invoces like 121432, 3436534 and 8989898 inside";
        Iterable<String> splitted = Splitter.on(" ").split(input);
Split does fixed length splitting, although you cannot give a different length for each “column” which makes it's use a bit limited while parsing some badly exported excels.
        String input =
                "A  1  1  1  1\n" +
                "B  1  2  2  2\n" +
                "C  1  2  3  3\n" +
                "D  1  2  5  3\n" +
                "E  3  2  5  4\n" +
                "F  3  3  7  5\n" +
                "G  3  3  7  5\n" +
                "H  3  3  9  7";
        Iterable<String> splitted = Splitter.fixedLength(3).trimResults().split(input);
You can use CharMatcher while splitting
        String input = "Some very stupid data with ids of invoces like 123231/fv/10/2010, 123231/fv/10/2010 and 123231/fv/10/2010";
        Iterable<String> splitted = Splitter.on(CharMatcher.DIGIT.negate())
                                            .trimResults()
                                            .omitEmptyStrings()
                                            .split(input);


Predicates / Functions

Predicates alone are not much, it's just an interface with a method that returns true, but if you combine predicates with functions and Collections2 (a guava class that simplifies working on collections), you get a nice tool in your toolbox.

But let's start with basic predicate use. Imagine we want to find whether there are users who have logins with digits inside. The inocation would be (returns boolean):
Predicates.in(users).apply(shouldNotHaveDigitsInLoginPredicate);
And the predicate looks like that
public class ShouldNotHaveDigitsInLoginPredicate implements Predicate<User> {
    @Override
    public boolean apply(User user) {
        checkNotNull(user);
        return CharMatcher.DIGIT.retainFrom(user.login).length() == 0;
    }    
}       
Now lets add a function that will transform a user to his full name:
public class FullNameFunction implements Function<User, String> {
    @Override
    public String apply(User user) {
        checkNotNull(user);
        return user.getFirstName() + " " + user.getLastName();
    }    
}
You can invoke it using static method transform:
List<User> users = newArrayList(new User(1L, "sylwek", "stall", "rambo"),
  new User(2L, "arnold", "schwartz", "commando"));

List<String> fullNames = transform(users, new FullNameFunction()); 
And now lets combine predicates with functions to print names of users that have logins which do not contain digits:
List<User> users = newArrayList(new User(1L, "sylwek", "stall", "rambo"), 
  new User(2L, "arnold", "schwartz", "commando"), 
  new User(3L, "hans", "kloss", "jw23"));

Collection<User> usersWithoutDigitsInLogin = filter(users, new ShouldNotHaveDigitsInLoginPredicate());
String names = Joiner.on("\n").join( transform(usersWithoutDigitsInLogin, new FullNameFunction()) );

What we do not get: fold (reduce) and tuples. Oh well, you'd probably turn to Java Functional Library anyway, if you wanted functions in Java, right?

CaseFormat

Ever wanted to turn those ugly PHP Pear names into nice java/cpp style with one liner? No? Well, anyway, you can:
String pearPhpName = "Really_Fucked_Up_PHP_PearConvention_That_Looks_UGLY_because_of_no_NAMESPACES";
String javaAndCPPName = CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL , pearPhpName);
Output: ReallyFuckedUpPhpPearconventionThatLooksUglyBecauseOfNoNamespaces

But since Oracle has taken over Sun, you may actually want to turn those into sql style, right?
        String sqlName = CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_UNDERSCORE, javaAndCPPName); 
Output: really_fucked_up_php_pearconvention_that_looks_ugly_because_of_no_namespaces

Collections

Guava has a superset of Google collections library 1.0, and this indeed is a very good reason to include this dependency in your poms. I won't even try to describe all the features, but just to point out a few nice things:
  • you have an Immutable version of pretty much everything
  • you get a few nice static and statically typed methods on common types like Lists, Sets, Maps, ObjectArrays, which include:
    • easy way of creating based on return type: e.g. newArrayList
    • transform (way to apply functions that returns Immutable version)
    • partition (paging)
    • reverse
And now for a few more interesting collections.


Mutlimaps

Mutlimap is basically a map that can have many values for a single key. Ever had to create a Map<T1, Set<T2>> in your code? You don't have to anymore.

Multimap<Integer, String> multimap = HashMultimap.create();
        multimap.put(1, "a");
        multimap.put(2, "b");
        multimap.put(3, "c");
        multimap.put(1, "a2"); 
There are of course immutable implementations as well: ImmutableListMultimap, ImmutableSetMultomap, etc.

You can construct immutables either in line (up to 5 elements) or using a builder:
Multimap<Integer, String> multimap = ImmutableSetMultimap.of(1, "a", 2, "b", 3, "c", 1, "a2"); 
Multimap<Integer, String> multimap = new ImmutableSetMultimap.Builder<Integer, String>()
        .put(1, "a")
        .put(2, "b")
        .put(3, "c")
        .put(1, "a2")
        .build();

BiMap

BiMap is a map that have only unique values. Consider this:
@Test(expected = IllegalArgumentException.class)
public void biMapShouldOnlyHaveUniqueValues() {
 BiMap<Integer, String> biMap = HashBiMap.create();
 biMap.put(1, "a");
 biMap.put(2, "b");
 biMap.put(3, "a"); //argh! an exception
} 
That allows you to inverse the map, so the values become key and the other way around:
BiMap<Integer, String> biMap = HashBiMap.create();
biMap.put(1, "a");
biMap.put(2, "b");
biMap.put(3, "c");

BiMap<String, Integer> invertedMap = biMap.inverse();
Not sure what I'd actually want to use it for.

Constraints

This allows you to add constraint checking on a collection, so that only values which pass the constraint may be added.

Imagine we want a collections of users with first letter 'r' in their logins.
Constraint<User> loginMustStartWithR = new Constraint<User>() {
    @Override
    public User checkElement(User user) {
        checkNotNull(user);
        
        if(!user.login.startsWith("r")) {
            throw new IllegalArgumentException("GTFO, you are not Rrrrrrrrr");
        }

        return user;
    }
};    
And now for a test:
@Test(expected = IllegalArgumentException.class)
public void shouldConstraintCollection() {
 //given
 Collection<User> users = newArrayList(new User(1L, "john", "rambo", "rambo"));
 Collection<User> usersThatStartWithR = constrainedCollection(users, loginMustStartWithR);

 //when
 usersThatStartWithR.add(new User(2L, "arnold", "schwarz", "commando"));
}
You also get notNull constraint out of the box:
//notice it's not an IllegalArgumentException :( 
@Test(expected = NullPointerException.class)
public void notNullConstraintShouldWork() {
 //given
 Collection<Integer> users = newArrayList(1);
 Collection<Integer> notNullCollection = constrainedCollection(users, notNull());

 //when
 notNullCollection.add(null);
} 
Thing to remember: constraints are not checking the data already present in a collection.

Tables

Just as expected, a table is a collection with columns, rows and values. No more Map<T1, Map<T2, T3>> I guess. The usage is simple and you can transpose:
Table<Integer, String, String> table = HashBasedTable.create();
table.put(1, "a", "1a");
table.put(1, "b", "1b");
table.put(2, "a", "2a");
table.put(2, "b", "2b");

Table transponedTable = Tables.transpose(table);
That's all, folks. I didn't present util.concurent, primitives, io and net packages, but you probably already know what to expect.

October 17, 2010

Java Developers' Day 2010 review

On the 7-8th of October 2010, Cracow held Java Developers' Day conference. This year it was two days long, so I guess they'll have to think about changing the name. My expectations weren't very high. First of all, I've heard an opinion that JDD is getting worse every year. Second, for the same price as GeeCON you got only one track. Third some of the lectures seemed really uninspiring.

For example, I was afraid, that the one about Flex is going to be the same one I've seen on 4Developers, half a year ago in Poznań. And, of course, there was the one most controversial to all the people I had a chance speaking with, the sponsored lecture from Wipro Technologies, titled: “Wipro in Europe and development opportunities on Polish market”.

Doesn't sound like something you'd like to listen to on a Java conference, does it? More like an advertisement, to me.

Fortunately, my doubts were mostly unfounded.

The first day started a little earlier than planned, with Bill Burke talk about RESTful Java. Quite nice, I must say, as long as you have no idea what REST means, as half of the lecture was very basic. The other part was about JAX-RS (and RESTEasy implementation), and that's where it got my attention. I haven't had a chance to use JAX-RS yet, but the simplicity and efficiency of it is very appealing. I'll have to give it a shot, especially when .NET/Java web service integration is sometimes very painful.

The second talk,  “Java programming in days of multi-core processors”, by Angelika Langer, was gorgeous. Maybe it's because of my limited experience in concurrency, maybe because Angelika presented a more in-depth view on the things happening under the hood, than I am used to, and maybe because she threw away the incorrect model most books present. What I'm sure about, is that Angelika is a great trainer and speaker, with vast knowledge and expertise.  It was a pure pleasure to listen to her and I can only hope to be so passionate and sharp at her age.

Not that she's that old, mind you. It's just that terms like “burned-out” and “tired” do not seem to have anything to do with her.


Then there was Jarosław Błąd from e-point, talking about performance tests in JEE. That was also quite nice, though a little too basic. I wish the speaker could show us a little more real case scenarios and stories, as it was obvious he had a lot of interesting thoughts on that matter, but probably because of NDAs, he decided to go with more theoretical and generic information instead. Anyway, this was a sponsored talk done right. Thanks e-point for not leaving us with just advertisements.

After the lunch came the hit of the day, Ted Neward talk about functional programming for busy developer. A few slides passed by, when Ted asked the audience, whether we would rather see presentation or life coding. Guess what the answer was.

The great thing about the lecture was that Ted didn't use anything more than standard Java, to show us the benefits of thinking in terms of functional languages. The examples were practical, with stuff you can really find from time to time in your code, and the advantages clear and explicit. Somewhere in the middle of the show, Ted said, that he wants us to remember, that we do not have to use anything fancy like Scala, to start solving some classes of problems in a much better way. I only wish he had more time on his hands, but I was lucky to sign in for Scala workshops with him on Friday.

I didn't go for Flex presentation, partially because of the beforementioned doubts, partially because I've met some friends and speakers on the way. I really wish I could be there, on their lectures, especially on Łukasz Kuczera talk about Lift+Comet and Łukasz Szydło presentation about Apprenticeship, but I could either do that or go for the workshop with Ted Neward, and after what Ted had shown us a few hours before, I was sure his workshop will be a mind opener.

And here is for all those anxious about just one track on JDD10. There were actually two on Friday, if you count the workshops, and even though that doesn't seem like much, the quality of what Ted had to offer, beat up the disadvantage of not being able to change every session for something different.

The last lecture on Thursday was “Brave changes: how to make new ideas happen”, given by Linda Rising. While not Java specific, that was quite interesting to me, mostly because of the latest changes I've been part of at TouK (both my own initiatives that you can read about here and here, and overall works on defining company goals and vision). Thing to remember: what your audience is eating is more important than what they are listening to. Scary but true.

Then there was the integration party. And as expected from programmers, Nintendo Wii had a much bigger take than girls :)

For three hours on Friday, I've been enjoying Ted Neward's Scala workshops. I won't give you much details, except it was really great, since Witek Wołejszo wrote a nice summary already.

And I didn't dare to go for “Wipro in Europe and development opportunities on Polish market”. I was afraid, that my positive experience from JDD10 could be a bit reduced.

Overall, another great conference. Thanks to Witek Wołejszo, Piotr Przybyłek and Tomasz Dziurko for this interesting trip.

October 13, 2010

Wicket form submit not safe for redirecting to intercept page

The problem

When you have a form, that anybody can see, but only logged on users can POST, you may want to redirect the user to the login page, and back to the form after login

Using wicket 1.3/1.4, if you do that using redirectToInterceptPage(loginPage) or RestartResponseAtInterceptPageException, after returning, the client will loose all the data entered to the form.

The details

The reason why this happens, is because of how redirectToInterceptPage works. It saves the URL of the requested page, and later, when continueToOriginalDestination is called, it redirects the client to the saved URL using GET. When the last call from the client was a non-ajax POST to the form, the client will be redirected without any posted data. Wicket will handle the situation issuing  HTTP 302 and redirecting the user again, but all the data is already lost.

The funny thing is that the data is actually getting to the form, because of the first POST, but then it's overwritten with nulls on the redirected GET. To make it clear, here's the HTTP conversation:

Client: POST http://localhost:8080/test?wicket:interface=:3:form::IFormSubmitListener:: (post to the form)
Server: HTTP 302 Moved Temporarily (the input was parsed, the model was updated, but you are being redirected to the login page because of redirectToInterceptPage or exception)
Client: GET http://localhost:8080/?wicket:interface=:4:::: 
Server: HTTP 200 OK (server is responding with the login page)
Client: POST  https://localhost:8443/j_spring_security_check.... (post login and password, here using spring security)
Server: HTTP 302 Moved Temporarily (validation is done. Now you are redirected from spring security to the page with wicket redirectToInterceptPage)
Client: GET https://localhost:8443/redirectAfterLogin  (here  redirectToInterceptPage is called)
Server: HTTP 302 Moved Temporarily (you are being redirected the original URL)
Client: GET http://localhost:8080/test?wicket:interface=:3:form::IFormSubmitListener:: (the same URL as the first POST but this time without post data. now your form is being submitted again, but with nulls instead of entered data)
Server: HTTP 302 Moved Temporarily (being redirected by wicket, because of Redirect After Post pattern)
Client: GET http://localhost:8080/?wicket:interface=:3:1::: (back on the form page)
Server: HTTP 200 OK (the form is empty by now)

As you see, if wicket would not redirect you at the end to the url requested by POST, but to the one called by last GET before the original POST, your data would be there.

The issue was reported two years ago. Doesn't look like it's getting fixed any time soon.

The walkaround

If you can require your users to be logged in before you show them the form, you are safe. If not, you can submit the form by AJAX. This will solve the problem, because wicket will recognize, that it cannot redirect the user to the AJAX POST target (<ajax-response> is not exactly what the user would like to have rendered in the browser), and will redirect with GET to the URL of the last page instead, which was also requested by GET. And since the data was converted to the form model in the first POST (line 1), all is well.

And in case you don't want to have a partial page update via AJAX, but would rather like to render the whole page, all you need to do is add setResponsePage(getPage()) to your button. For example like this:


    class AjaxSendButton extends AjaxFallbackButton {
        public AjaxSendButton(String id, Form form) {
            super(id, form);
        }

        @Override
        protected void onSubmit(AjaxRequestTarget target, Form form) {
            //process your form input here
            setResponsePage(getPage());
        }
    }

Now your ajax form behaves just like a non ajax form, but can be redirected to an intercept page

The catch

When submitting forms via AJAX you have to be aware, that your form may be submitted without your submit button being clicked on. This may have unforseen consequences. For the whole problem description and a solution go here

October 5, 2010

Video from my presentation at Agile Warsaw

Here's the video from my presentation and the discussion about Agile Skills Project and our experiments in motivating developers at my company, that I had a chance to show at Agile Warsaw.

Do not ask my why the camera is 100% time focused on the wall, I have no freaking idea :) The voices are there, and that matters.

You can either watch it on Parleys: http://parleys.com/d/2019 or right here. Be warned: it's in Polish.

September 21, 2010

Getting rid of null parameters with a simple spring aspect


What is the most hated and at the same time the most popular exception in the world?

I bet it's the NullPointerException.

NullPointerException can mean anything, from simple “ups, I didn't think that can be null” to hours and days of debugging of third-party libraries (try using Dozer for complicated transformations, I dare you).

The funny thing is, it's trivial to get rid of all the NullPointerExceptions in your code. This triviality is a side effect of a technique called “Design by Contract”.

I won't go into much details about the theory, you can find everything you need on Wikipedia, but in the nutshell Design by Contract means:
  • each method has a precondition (what it expects before being called)
  • each method has a postcondition (what it guarantees, what is returned)
  • each class has an constraint on its state (class invariant)

So at the beginning of each method you check whether preconditions are met, at the end, whether postconditions and invariant are met, and if something's wrong you throw an exception saying what is wrong.

Using Spring's internal static methods that throw appropriate exceptions (IllegalArgumentException), it can look something like this:
import static org.springframework.util.Assert.notNull;
import static org.springframework.util.StringUtils.hasText;

public class BranchCreator {
    public Story createNewBranch(Story story, User user, String title) {
        verifyParameters(story, user, title);
        Story branch = //... the body of the class returnig an object
        verifyRetunedValue(branch);
        return branch;
    }

    private void verifyParameters(Story story, User user, String title) {
        notNull(story);
        notNull(user);
        hasText(title);
    }

    private void verifyRetunedValue(Story branch) {
        notNull(branch);
    }
}

You can also use Validate class from apache commons instead of spring's notNull/hasText.

Usually I just check preconditions and write tests for postconditions and constraints. But still, this is all boiler plate code. To move it out of your class, you can use many Design by Contract libraries, for example SpringContracts, or Contract4J. Either way you end up checking the preconditions on every public method.

And guess what? Except for Data Transfer Objects and some setters, every public method I write expects its parameters NOT to be null.

So to save us some writing of this boiler plate ocde, how about adding a simple aspect that will make it impossible in the whole application, to pass null to anything other than DTOs and setters? Without any additional libraries (I assume you are already using Spring Framework), annotations, and what else.

Why would I like to not allow for nulls in parameters? Because we have method overloading in modern languages. Seriously, how often do you want to see something like this:

Address address = AddressFactory.create(null, null, null, null);


And this is not much better either

Microsoft.Office.Interop.Excel.Workbook theWorkbook = ExcelObj.Workbooks.Open(openFileDialog.FileName, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing, Type.Missing);

The solution

So here is a simple solution: you add one class to your project and a few lines of spring IoC configuration.

The class (aspect) looks like this:
import org.aspectj.lang.JoinPoint;
import static org.springframework.util.Assert.notNull;

public class NotNullParametersAspect {
    public void throwExceptionIfParametersAreNull(JoinPoint joinPoint) {
        for(Object argument : joinPoint.getArgs()) {
            notNull(argument);
        }
    }
}

And the spring configuration is here (remember to change the namespace to your project).
<aop:config proxy-target-class="true"> 
    <aop:aspect ref="notNullParametersAspect">
        <aop:pointcut expression="execution(public * eu.solidcraft.*..*.*(..))
                          &amp;&amp; !execution(public * eu.solidcraft.*..*Dto.*(..))
                          &amp;&amp; !execution(public * eu.solidcraft.*..*.set*(..))" id="allPublicApplicationOperationsExceptDtoAndSetters"> 
            <aop:before method="throwExceptionIfParametersAreNull" pointcut-ref="allPublicApplicationOperationsExceptDtoAndSetters"></aop:before>     
        </aop:pointcut> 
 
        <task:annotation-driven>
            <bean class="eu.solidcraft.aspects.NotNullParametersAspect" id="notNullParametersAspect"></bean>
        </task:annotation-driven>
    </aop:aspect>
</aop:config>

The "&amp;&amp;" is no error, it's just && condition escaped in xml. If you do not understand aspectj pointcut definition syntaxt, here is a little cheat sheet.

And here is a test telling us that the configuration is succesfull.

public class NotNullParametersAspectIntegrationTest extends AbstractIntegrationTest {
    @Resource(name = "userFeedbackFacade")
    private UserFeedbackFacade userFeedbackFacade;

    @Test(expected = IllegalArgumentException.class)
    public void shouldThrowExceptionIfParametersAreNull() {
        //when
        userFeedbackFacade.sendFeedback(null);

        //then exception is thrown
    }

    @Test
    public void shouldNotThrowExceptionForNullParametersOnDto() {
        //when
        UserBookmarkDto userBookmarkDto = new UserBookmarkDto();
        userBookmarkDto.withChapter(null);
        StoryAncestorDto ancestorDto = new StoryAncestorDto(null, null, null, null);

        //then no exception is thrown
    }
} 
AbstractIntegrationTest is a simple class that starts the spring test context. You can use AbstractTransactionalJUnit4SpringContextTests with @ContextConfiguration(..) instead.

The catch

Ah yes, there is a catch. Since spring AOP uses either J2SE dynamic proxies basing on an interface or aspectj CGLIB proxies, every class will either need an interface (for simple proxy based aspect weaving) or a constructor without any parameters (for cglib weaving). The good news is that the constructor can be private.

September 6, 2010

Agile Skills Project at my company

Unfulfilled programmers


Erich Fromm, a famous humanist, philosopher and psychologist strongly believed that people are basically good. If he was right, then either our society is a mind-breaking dystopia or we have a great misfortune of working in a field that burns people out, because many IT people I know are more like Al Bundy than anyone else.

Why is being a couch potato something wrong? Happiness can be achieved in many different ways, but not by passive pleasures. One way of pursuing happiness is by self realization and while self realization can happen in any activity, it's makes perfect sense to have it at work, where you spend one third of your life time anyway.

But many developers I know, consider work as something boring at best, dreadful at worst. True, programming can be awful, when you have to dig deep into a terrible code base without any perspective for a change, but IT is vast and you can always find something interesting, and once you learn it, you will find a way to make money on it, either by changing position inside your company or changing your employer altogether.

Yet most unhappy IT professionals don't do anything to change their situation. The main reason for that is, because it requires a lot of learning, and learning at home is not the most beloved activity for a couch potato.

So why are developers turning into couch potatoes in the first place? Why the last thing a typical developer will do back home is learning and polishing his skills? There are plenty of reasons for that.

The three main roots of an IT couch potato


First, our work is tiresome. Nearly every job offer you can find mentions “able to work under pressure” and “flexible long working hours” in the requirements. This translates directly to the “burn-out” phenomenon.

Second, the technological landscape is changing overwhelming fast. Unless you work for a slowly adapting institution like a bank, your skills will be outdated in a few years time. Sure the deceased Sun, god help us, granted Java developers four years of relative stagnation, but that's an exception and it's going to end soon enough anyway (unless, of course, these are just convulsions before slow death of technology). You better learn and you better learn fast, or you'll have no other option than to promote yourself into management.

Third, just how long can you sit by your computer everyday? Yeah, I know, some people spend years playing WoW, Eve and alike, barely moving. I am a sinner myself, with Steam reporting over 350 hours in Modern Warfare 2, 200 hours in F.E.A.R. 2 multi, and countless months of my life wasted by Sid's Civilization. But for not-addicted, it's just simply stupid, not to mention unhealthy, to have your ass integrated with the chair. No matter how comfortable it may be. There is more to life than that.

Case Study at my company


It all started with a few SQL programmers grumbling about how they are bored to death, and how they would like to switch to OO programming. I'm not a person who waits, so next thing I did was asking our management if they could move those guys to Java/C# projects. And the management was all for it, with just one requirement: they would have to first learn our technology stack at home, not to be totally lost and unproductive. After all, the more technologies an employee know, the more valuable he is for the employer (think about switching people between projects).

A few months later and nothing has changed. I'm asking sql guys how the learning is going, and I get the answer: it hasn't started yet.

Now, I know the best way to learn something is by hands-on experience at work. After all that's why I've been changing my job a few times: to have a real world experience. It's easier to learn french if you move to Paris. And learning at home is hard because of the aforementioned reasons. The very same reasons, why you get only 650 people on a free conference, like Javarsovia.

So what can we do, then? How about we remove all the obstacles? How about we make learning at home fun, satisfying and profitable. How about we provide  motivation and feedback. How about we also solve the never-ending dissonance between employee's financial and employer's productivity expectations on the way. Sounds interesting? Let's try, then.

First: make it profitable.


Up to some point, people get motivated by money. It won't work if you are already earning enough to pay for everything you need, but in a country like Poland, to be able to build/buy yourself a house, you have to be making many times the average salary. So here, money is still a major motivator. Every year, every developer goes back to his boss and says: I want more.

Guess what, your boss wants to pay you more. No kidding. After all Henry Ford's said:
“There is one rule for industrialists and that is: make the best quality of goods possible at the lowest cost possible, paying the highest wages possible”.

Highest wages. You boss really wants to pay more. But to stay in business, the company needs you to either improve the quality or productivity. Both mean more money to the business, and more money to pay you with. If you consider that, the goal of a developer who wants to learn something new (or get better at something) is on the way to make the company more profitable. After all, this is software development – you never know what technology you gonna need tomorrow.

This works both for completely new stuff, and for learning something that company is already quite good at. If you know several technologies that the company is working with, you are more valuable, because you can handle more projects (you boss may think in terms of reallocating resources).

After thinking about all of this I went to my bosses and asked them: will you pay more to the people who learn different technologies at home? Even when they can't use them right now at work? Will you give a rise to those Oracle guys, if they learn Java?

The answer was: definitely! They actually said that every time a developer asks for a rise, they ask him back: what have you done in the last year to improve your market value? What have you learned? Because every time an employee's market value increases, the company's value increases.

Simply speaking: more skills means more money. Both for the developer and for the company. It's amazing, how often people forget about it. Making it crystal clear can give you a motivational boost to do something at home and a nice perspective. You don't have to switch your job to get a rise. You need to learn more, and they'll happily pay you more.

Second: make it easy


The main question with learning is where to start from? And for software developers it's the most important, most difficult question, because there is no way to learn everything, because spending years studying can be simply a waste of time, if the technology is dead/outdated the moment you get productive with it. What should I learn or why should I learn at all, if the risk of wasting the most precious thing in my life, my time, is so high? How do I decide what to learn.

Lets make learning safe and easy.

Your best bet is to start with something that has much longer life expectancy, something that will help you right away, no matter what a technology you have to work with in your next project. And something that is relatively simple to learn: agile skills.

These are skills of an agile developer, well established, well recognized, and not going away any time soon, because we still do not have anything on the horizon that could surpass them.

Yeah, I know, the world 'agile' is so popular nowadays, that even my grandma is agile, but lets go for a solid list of things agile. No bullshit theory, no marketing mumbo jumbo, give me a precise, distilled and refined list of things I should learn, things that will help me, things worth spending my time on.

Here it is:

The Agile Skills Project


The project is all about self improvement and learning. It's a great inventory of “ isolated, learned, practiced, and refined” agile skills, with definitions, resources, descriptions of steps to mastery and success stories. Take a look at the “Pair Programming” page, for example.

All the skills are divided into different areas: Business Value, Collaboration, Confidence, Product, Self Improvement, Supportive Culture, Technical Excellence. You even get a nice mind-map with it.

This is a single reference point for all those who do not know where to start or where to go next. All these skills are in high demand on the market and with a very long Time-To-Live. The best thing is though: no matter what technology you gonna work with tomorrow, you can benefit from them.

I took the list from the website, tidied it up a bit, refactored it for the needs of my company, and proposed it as a Request For Comment, a wiki page, where everyone gets to discuss and shape up the idea, before we give it to the management.

Soon we had a discussion. It wasn't easy to make everyone understand the concept, but after a while people joined in, and we added some more stuff.

Level up!


The Agile Skills Project is more than a simple index. It tries to create a learning ecosystem, by defining quests:
"Quests" are on-the-job experiments, self-assessments, peer-reviews, course experiences or other activities intended to help a person better apply a particular agile developer skill set.

It's a bit like a Role Playing Game. You have your quests, you do them, you get experience. For experience you get more money and new toys (technologies) to play with.  It's fun. Billions of MMORPG players cannot be wrong.

But to make that happen we need something every game has: feedback.

Third: give feedback


OK, so we have a bit of motivation (money) and a list of goals (agile skills). Who is going to give us quests, and who is going to tell us we did a good job? How will we have our feedback?

The first and most important thing, is to see the results of you actions. Otherwise you loose focus and motivation (money can only get you that far). Therefore you should create a list of quests you have done. Put it on the intranet or somewhere, where you can show it to others. It's important, because you are going to share it with your mentor.

Yes, a mentor. Choose someone from your company, someone you trust, someone you respect. It doesn't have to be an Einstein. Meet with this person once a month, during your work-time. An hour should do. Discuss with your mentor what quests you want to accomplish this month. Could be anything, reading an IT book, learning new programming language or taking another step to master one of the agile skills from the list. Tell your mentor when you'll be done. Meet together again next month, and either put the quest in your done-list, or mark it as 'failed'.

The role of the mentor is to listen to you, remove obstacles, help you choose a good path and give you feedback.

You'll be surprised by how much the meeting with your mentor motivates you. It works much better than money: you don't want to fail in the eyes of the mentor, because this is the guy you respect, and you want him to respect you as well. And once you see your constant improvement by filling the list of quests done with your mentor, it gets addictive.

Smells corporate?


How is this any different to what you can sometimes see in a corporation, with a year long plan of tasks your boss is giving you to accomplish to get your bonus?

Well, first of all, these will be your quests, chosen by you. Second, you will choose your mentor as well. Your boss usually doesn't know a thing about what you are doing. Third, it's all about your self-improvement, not meeting some company goals. You get better at something, the company gets better at something. After all a company is not much more than the people working at it.  Fourth, it's a fast feedback cycle, you do not have to wait till the end of the year to get it.

And finally, it may be a bit corporate, because I have never seen any small company doing anything like this. But even if it is, it still seems like worthwhile. Anything to get me out of the couch.

Discuss


It's a bit too early to tell whether the idea will be successful. We have just started. Fo me it is already helpfull, because with a list of quests done I have have a feeling of progress. If you'd like to discuss this, and other ways to animate software developers to do something more, I'm leading a meeting at Agile Warsaw group about it, on the 20th of September, 19:00. Feel invited.

By the way, here you have a trial of "other ways to animate" from our internal TouK Code Jam Party, we held a week ago. Doesn't look mych corporate, does it? 

July 6, 2010

NYAC & COOLuary 2010 review

NYAC stays for Not Yet Another Conference, and it gives a promise I was happy to verify. The event took place in Cracow on 19th-20th of June, thanks to Grzegorz Duda. Tough timing I'd say, just a week before Javarsovia, but the audience was supposed to be small, so that wasn't a problem. NYAC was actually two things: a one day conference with carefully chosen speakers that got a bit more time than usually, and another day of an unconference.

If you didn't have a chance to participate in an unconference (Open Space Technology), it's totally people driven. Everyone is able to start a meeting, and it's usually a discussion, not a lecture.

But let's start from the beginning. I got there on Friday evening, and my luck was definitely with me, as I bumped into Paweł Lipiński at a gas station. He warned me about the post-soviet hotel, so we walked back carrying loads of beer, that helped shield us from the ugly face of socrealism and it's terrible architecture. Well defended, together with Wojtek Erbetowski, we spend the late hours enjoying the drink and a smalltalk.

The first day started.

So what's so different about NYAC, you ask. First day is just a conference, after all. True, but Grzegorz had a great idea, to make a survey about the best polish java speaker. He used the outcomes to invite some of the most interesting people in our field. He asked them to make a few lectures, one after another, and he gave them more time than usually (which turned out to be not enough anyway), and a much smaller audience.

The effect?

Well, it's like a difference between a stadium-sized concert, and a jam session in a private house.  The second one allows you to be more involved, and get much closer to the source of fun. Is it the way to do conferences? Yes, but only if you have the top guys out there. And that was the case. Have a look at the agenda, the only problem you could have is to decide which session to choose from.

Fortunately, being here and there, I've already seen some of the presentations, so it was a bit easier to me. Sławomir Sobótka had three great talks about Craftsmanship, Domain Driven Design and JPA/Hibernate traps (we all know that JPA 1.0 was a big step backward, compared to pure Hibernate, but Sławek had some really great examples of the most popular pitfalls). Szczepan Feber with Łukasz Milewski gave us a handful of good tips about continuous integration. Igor Czechowski talked a bit about his nightmares with Maven. Paweł Lipiński warned me that I may not find anything new in his talks, so I skipped them, but I really wish there was a camera, since Paweł is a great speaker and it's always refreshing to see his enthusiasm.

All together it was a condensed, great event, that I've left with a nice set of notes and a mind full of thoughts.

Then there was a party, though Grzegorz didn't give the address, just the name of the pub, and as with all the names, it was easy to forget and hard to find using GPS.

Great aspect of going to so many conferences (I've been to six this year) is that eventually you get a nice pack of people to drink with, but with such a small scale (there was something about 50 attendees) it's easier to get to know everyone and not feel lost in the crowd anyway.


Some people stayed till like 3am, but since I was kind of on a mission gathering/crunching knowledge, I decided to get back at midnight.

The second day was all about self organized groups of talkers. We had a flipchart where everyone could post a proposal, then we grouped together similar topics, and sorted them out considering whether the guys we wanted to join in, were leaving early or waking up late. Seriously. After all, talking about DDD without Sławek or about A/B/TDD without Paweł would be like wasting resources.

The only “real” presentation that day, was a sponsored talk from Tomasz Skutnik (e-point) about class loaders in JEE, and it was unexpectedly good, I must say.

I also had a chance to animate a smalltalk, which I used to confront and verify some of the ideas I've been preparing for my Javarsovia presentation. The feedback was good, and some of the points from the discussion helped me to refactor my final talk.

All together it was a great weekend, and I can recommend NYAC/COOluary with my heart. It's a moving conference, every year in a different city, so stay tuned to Grzegorz's site, and don't forget to visit the event whenever you have a chance.


May 18, 2010

GeeCON 2010 general impressions

GeeCON 2009 in Cracow was a wonderful experience, and this year Poznan held the conference. If you ever wonder whether GeeCON is worth the time and money, the answer is plain and simple: definitely yes.

I'm not able to give you a full review this time, as I'm leaving tomorrow for Paris, and I haven't prepared myself yet, but let me summarize the most important topics.

There were two different presentations about Erlang way (Actor and Agent models) of solving the problem with concurrency. One was a general overview of the problem and possible solutions running on JVM, and the other was about Akka Project/Platform that looks very interesting and above all, is simple to use (at least from what we've seen at the presentation). So far I've been trying to shield myself from concurrency problems with web servers, stateless services and optimistic locking on database level, but the solution presented here is really appealing. I'd love to try it out.

Joonas Lehtinen had a very nice lecture about Vaadin. There was a talk about Hades which is probably going to replace the solution I usually use (hibernate-generic-dao). Kuba Kurlenda is testing it on real project right now.

Stephan Herrmann was talking about an evolutionary idea for Object Oriented Programming:  Object Teams. This may be the next big step we need, but it would have to gain some momentum and find a real life use in a big project. Unfortunately Object Teams do not yet have a successful example to follow. The main question I see is whether it's worth investing the time of your team, while everyone already knows about design patterns, which solve similar problems.

There was a presentation about Spring Roo framework which is something you should know before you move to Grails or Ruby on Rails. Even if you do not plan to do it, it's a great way to create prototypes.

Waldemar Kot had a very condensed, content rich but easy to learn lecture about Complex Event Processing. It was definitely worth listening to. I'll try to infect my SQL friends with this idea, as it seems like something they would really like to do, when they move from PL/SQL to Java. Thomas Sundberg repeated his talk about Software Craftsmanship that he gave at Agile Central Europe. There was a little bit about Comet (push style Ajax), agility, OOP and lots of other interesting stuff.

And there was a great party, but that's a completely different story.

If you have a chance to go to GeeCON 2011, do not hesitate to.

PS: all the pictures come from the official GeeCON gallery. I don't know about the authors, though.

May 17, 2010

Open Code Review

Łukasz Lenart at Warszawa Design Patterns Study Group gave a very interesting presentation about Open Code Review in his company (you can read the details here). I've failed to notice whether it's his own idea, or somebody's else, but that's not as important as the fact how well it works.
Open Code Review, in short, is when a small group of programmers (up to 6) meet once in a while for a brainstorming session, take some printed out code, and try to find out, for an hour or so, what could be done better.
It's basically about sharing knowledge and insight with others. It can be a very beneficial experience, especially for teams that do not use Pair Programming, or to pairs that do not switch very often, but I think anyone can find it useful.

The power of this technique is reflection. When we code, when we create software, we often find ourselves thinking in a specific way. We tend to take things for granted. We start to look at the code with our mind set on a particular solution, and as a result, we can't see the forest for the trees. This is how human mind works, it's nothing new. After a few weeks of walking the same route, you stop noticing the buildings along the way.

It's also one of the reasons why Pair Programming is so efficient. “You cannot solve a problem with the same mind-set that got you into the problem in the first place”, as some believe Einstein said. Having another mind next to you, looking at the same code, gives a chance to spot the problem as it is being created, but more importantly, it is very helpful in the process of discovering the best design. Open Code Review works in a similar way. It allows you to learn different mindsets and different styles of solving problems. Nothing is as refreshing as being able to see from other points of view.

Apart from this, OCR meetings also help in spreading knowledge (people read different books) and experience (people work on different projects with different technologies and problems).

This idea inspired me to organize similar meetings at TouK and I've got very good feedback so far. Our formula crystallized by looking at the the past and finding out what was cool and what was not. Our take on Open Code Review is quite different to what Łukasz proposed.

But first, things that we do the same way: we take maximum 6 people (plus the moderator), we gather them up in a conference room, we hand out code printed on paper.

The code we take is not Open Source, as Łukasz suggested. His reasoning was, that if you take somebody's code, it could turn personal, we all put some heart into our work, after all. This is where we disagree: we have found it very valuable to take internal code from our projects. We are not afraid of our code being criticized. As Robert C. Martin wrote: every good programmer should get used to professional code review. Bad programmers can benefit from it even better. And if you have some Prima Donnas in your team, unwilling to hear constructive critique to their code... screw them. Nobody's able to work with them anyways.

Second thing we do differently, is that we do not comment the code on paper. We actually have a working environment (IDE and stuff) with three keyboards/mice, thanks to USB hubs. We either comment the code directly, writing TODOs, for bigger changes that would take more time than it's left, or we do the refactoring right away. This allows us to make a commit to the repository after the session and improve some project a bit. It's also much easier to talk about the code, when you have a keyboard at hand. Sometimes it's just way simpler to show what you mean than to explain it with words.

We also take a bit longer for each meeting: usually an hour and a half, or two hours. This is the optimum time, to learn and do something meaningful, and not get tired. And we do not split attendees into teams, we want people to talk a lot, and we want people to talk with each other.

So what is our recipe for an Open Code Review? It's like this:

Ingridients
  1. Invite anyone who wants to participate, from all over company, but take no more than 4 to 6 programmers plus a moderator
  2. Take a conference room with a laptop, three or four keyboards/mice, and a projector
  3. Set up the meeting for 1.5 or 2 hours
  4. Moderator prints out a class (or a few classes) that
    1. has between 150 and 300 lines of code
    2. has unit/integration tests for all non trivial methods
    3. is from a project that works with our environment (IDE/Maven etc.)
    4. is not trivial and not completely shitty (that would be too easy and nobody will learn anything)

Instructions
  1. Give everyone printouts and 10 to 15 minutes to read the code and make notes
  2. Ask whether you should start top-down or from some other point in the code (some classes are broken conceptually and it's better to fix them by talking about their responsibilities first). If in doubt start from the top.
  3. For every block/method/property ask whether anyone thinks that it could be done better. The person that has the idea, has the power (keyboard) to do it.
    1. If it's a small change, fix the code
    2. If it's a big change that would take longer than the meeting, write a TODO
    3. If someone disagrees, talk, talk, talk until consensus is found or you agree that you disagree and move on
    4. Run (write if needed) tests, to make sure the code works. This is a real refactoring, you cannot break the code. Broken code is not considered better.
  4. If you cannot find anything more that you could get better, use the principles from “Clean Code” (or some other book) to guide you.
  5. At the end of the meeting, commit the code.

Things to remember

If you are the moderator, remember to shut up and keep quiet. You probably had a bit more time to read the code so it's natural you'll have noticed more flaws, but let the people do the exploration and discovery. When they don't have anything else to say, then it's the time for you to speak.

If you are the author, no matter what, DO NOT EXPLAIN the code. Either the code speaks for itself, or is badly written.

Enjoy.

PS: this practice has been unfortunately stopped, because I've started to go to conferences and could not moderate any more. Do not make my mistake and communicate clearly, that ANYONE can call for an Open Code Review, and that the moderator should be changed every two weeks or so. I've failed at doing that, and now I have to revive the whole thing.

May 12, 2010

Boy Scout Rule in practice

I've started doing Scrum in late 2008 (thanks to Piotr Burdyło). And though I've immediately seen benefits of practices like daily standups, sprint planning, reviews and retrospectives, I had a big problem with finding out how to deal with refactoring within this method.

Before Scrum, whenever I found a place in code that I thought should be done better, I'd put a TODO comment. I would return to these comments whenever I saved some time on what I was doing, or when I decided that it has already been too ugly for my standards. And since my standards are pretty tight, I've been  (and making others) refactoring quite often and it worked well for me.

When doing code review, I'd write TODOs, discuss them with the authors of the code, and make them do clean up as the top priority.

But what were my options in Scrum?

1. Don't tell anybody, and just use your left over time to clean up the TODOs

This is against transparency, which Scrum is advocating, and against my moral backbone (no lying). And since you are evaluating tasks/features better and better, sooner or later you'll have no left over time for TODOs anyway. Ergo: this won't do.

2. Create additional tasks/features for refactoring

These are not visible from the product owner point of view, so some Scrum Masters are strongly pushing against them.

Ok, so I had to choose the second option, and make my Scrum Master live with it. But then, another problem emerged: unless you put a cleaning task for every class that has TODOs inside, sooner or later you'll end up with tasks like “Do all the TODOs in my Big-Ugly-Module”.

This will happen because doing TODOs may have very low priority, because people in a hurry don't bother doing them, and because some people don't write TODOs at all, rather thinking: “yeah, this is ugly, lets fix it, when we fix the rest of the module”.

So it seams that your code is predestined to go down the great spiral of (bio)degradation, until your team can't stand it anymore and somebody asks for a Scrum task to clean up the mess.

Is there a solution to this? Is there a third way?

Sure there is.

What if we can switch the direction the code quality goes in? What if our code can get better with time, instead of getting worse? That would be cool and it would solve our problem with TODOs in Scrum.



Boy Scout Rule

Leave every code you read in better shape than you found it.

Ha! That sounds like something everyone would sign under, and completely forget about a few minutes later, right? I mean, phrases like this sure look nice, but the real battlefield, the front-line, is a mean and messy place, right? You sit in your trench, trying to survive and keep the project alive, while bullets fly above your head. How can you try to make things better, when you couldn't keep them clean in the first place? No way to do it, man, there is too little time, right?

Wrong.

Let me show you how to use this rule in practice, without any significant time burden.


Real case scenario

You are in the middle of writing an application, you take a task from the Scrum white-board. It's a new feature. It says: “When importing orders, every shipping address should be verified. If it's not in our shippable zone, it should be canceled (status change + send an email to the frontend application)”.

Ok, so you want to write a new acceptance test, for this feature. But first you need to know what to test, which part of the application is responsible for importing orders. You search the project structure, and you spot the OrderService interface. You nose tells you, that it should be somewhere there. You take a look.

public interface OrderService {
    /**
     * Imports orders.
     */

    public void importOrders();
    /**
     * Export orders.
     */
    public void exportOrders(DateTime date);

    /**
     * Create raport of placed orders
     */
    public OrderRaport createRaport(DateTime date);
}


Yeah, it's there. The importOrders() method. You search for implementations, and as you expect, there is an OrderServiceImpl.


WTF is with this blur, you ask. It's intentional. It's how much you see during your first second of looking at the code. At the first second you, as a good writer, have already noticed two things:
  1. The method has 30 lines. It's probably too long. Good method should be as short as possible. This, according to Robert C. Martin, means between 1 and 7 lines.

  2. The are several different BLOCKS of code.
What does the second thing mean? It means, that the author has noticed the first thing as well, so he decomposed the method into different blocks, by putting an empty line between them.

This is so natural, so popular, nobody thinks about it anymore. We put empty lines between different blocks of code, to help us read the code. When we read the code, we can skip such blocks, because we already know, what those blocks do.

We put together similar stuff, so we can put a name into our mind-map, that given block does something. Next, we can fast read and skip it consequently.

There are two big problems with such method:
  1. it requires you to read all the details (every line) of the code at least once, before you put names of those blocks in you mind

  2. it is not the way Object Oriented Programming is supposed to separate stuff that make a single concept

So how about fixing it, while we read it? We have to read and understand it anyway. How about not keeping the names of blocks in our memory, but putting them into the code instead.


Comment every block of code

You are doing it anyway, but instead of saving the names/explanations in your head, save them in the code for a moment.

So our code will look like this:

@Override
public void importOrders() {
//create date before importing from web service
    DateTime date = new DateTime();

//import data transfer objects from web service
    List<SalesOrderExportDetail> orders = salesOrderWS.getSalesOrdersFromWebService();

//create summary for importing orders
    SalesOrderImport salesImport = new SalesOrderImport();

//for each order try
    for(SalesOrderExportDetail salesOrder : orders) {
        try {

//convert it to domain object
            Order order = new Order();
            order.setValue(new BigDecimal(salesOrder.getValue(), new MathContext(2, RoundingMode.HALF_EVEN)));
            order.setAddress(convertToAddress(salesOrder.getShippingAddress()));
            order.setBuyerAddress(convertToAddress(salesOrder.getBuyerAddress()));

//calculate vat
            double vat = calculator.vatRate(order.getAddress());
            order.setVatRate(vat);
            BigDecimal value = calculator.value(vat);
            order.setVatValue(value);

//calculate value in PLN
            Currency curr = order.getCurrency();
            if(curr != Currencies.PLN) {
                order.setPln(converter.convert(curr, order.getValue()));
            }

//save in repository
            orderRepository.save(order);

//prepare information whether it was succesfull                
            salesImport.imported(order);
        } catch(Exception e) {
            salesImport.notImported(e);
        }
    }

//save the summary for importing orders
    salesImport.setDate(date);
    orderRepository.save(salesImport);
}

Put the comments without any indentation, so that you won't forget to get rid of them, and they won't mix with other comments in the code. Make those comments verbose, make them say everything that is important on this level of abstraction. You are a programmer, you probably write faster than you think, so there was no additional time spend on this action.
Now for the second step.


Eliminate blocks of code using explicit names

The goal is to compress all the information from inside the block and the comment into one-liner. That saves the time the reader needs to get the meaning of the code next time. It usually means, replacing the block with a method, with a name created from the comment. So for example this:
//calculate vat
    double vat = calculator.vatRate(order.getAddress());
    order.setVatRate(vat);
    BigDecimal value = calculator.value(vat);
    order.setVatValue(vat);

will be replaced by this:
order.calculateVat();
Who said that the new method has to be in the same class? In this situation, it makes much more sense to put the use of the calculator into the order, after all it has all the data (we are using aspectj load time weaving to get the calculator into the order, but there are plenty of other methods if you don't want to use aspectj).

And this:
order.calculateVat();
//convert it to domain object
    Order order = new Order();
    order.setValue(new BigDecimal(salesOrder.getValue(), new MathContext(2, RoundingMode.HALF_EVEN)));
    order.setAddress(convertToAddress(salesOrder.getShippingAddress()));
    order.setBuyerAddress(convertToAddress(salesOrder.getBuyerAddress())); 

Will be replaced by this:
Order order = orderConverter.toOrder(salesOrderDto);
Here we have decided to put the body inside a new class: OrderConverter, because the convertion is a serious responsibility and the OrderService look like it's got other things to do already (Single Responsibility Principle).

Even one-liner can be a bit fixed, for example in this part:
//create date before importing from web service
    DateTime date = new DateTime();
The comment is put, because the name of the variable does not communicate it's intention. We can get rid of the comment if we give the necessary information to the reader in other way. For example like this:
DateTime dateImportStartedAt = new DateTime();
We should eliminate every block and split every part of the code that serves a different need. Even for-each loops and try-catch expressions. For-each responsibility is to iterate over a collection, and call someone else to do the job on an element. Try-catch is responsible for handling an error, so somebody else should do the unsafe job for it.

This is the Single Responsibility Principle on a method level. Make every method have only one reason to change, and your life (and the readability of your code) will be better.

Since these are very basic and easy refactoring steps, that your IDE does automatically with little effort from you, the time burden is insignificant. You had to understand the class anyway, and on the way, you've made it better, easier to understand. Now the part of your class responsible for imports looks like this:
@Override
public void importOrders() {
    DateTime dateImportStartedAt = new DateTime();
    List<SalesOrderDto> salesOrderDtos = salesOrderWebServiceClient.getSinceLastVisitOrderedByNumber();
    ImportSummary importSummary = tryToImportEach(salesOrderDtos, dateImportStartedAt);
    orderRepository.save(importSummary);
}

private ImportSummary tryToImportEach(List<SalesOrderDto> salesOrderDtos, DateTime dateImportStartedAt) {
    ImportSummary importSummary = new ImportSummary(dateImportStartedAt);
    for(SalesOrderDto salesOrderDto : salesOrderDtos) {
        importSummary.add(tryToImport(salesOrderDto));
    }
    return importSummary;
}

private OrderImportOutcome tryToImport(SalesOrderDto salesOrderDto) {
    OrderImportOutcome orderImportOutcome = null;
    try {
        orderImportOutcome = importOne(salesOrderDto);
    } catch(Exception e) {
        orderImportOutcome = new OrderImportFailure(e);
    }
    return orderImportOutcome;
}

private OrderImportSuccess importOne(SalesOrderDto salesOrderDto) {
    Order order = orderConverter.toOrder(salesOrderDto);
    order.calculateVat();
    order.updatePlnValueIfNeeded();
    orderRepository.save(order);
    return new OrderImportSuccess(order);
}

Every method has a meaningful name. No more than 7 lines of code per method. You don't need to divide code into blocks, methods are an object oriented solution to divide the code. By the way, you've delegated some responsibilities to other classes. You've refined the code, made it more precise, more clear, more readable.

But somewhere in your head there is a voice saying: “Hey, I had just one, 30-lines method, now I have four, taking the same space. If I do it to other methods, I'm gonna have like 15 methods per class. I'll get lost, it's no way easier for me”.

But that is, because you have one more big mistake to fix.


The name is the responsibility

Have a look at the name of the interface: OrderService. Can you tell me what this class is responsible for? Every class should have just one responsibility, what is it for OrderService?

Everything about orders?

What kind of responsibility is “everything”? There is no such thing as “everything”? Thinking this way you can create single class named God-Emperor and end your OOP right there. You'll never need other classes anymore.

No, my friend. The name SomethingService is a great mistake, just like a SomethingController or SomethingManager. I know quite a few SomethingManagers in real life, that you wouldn't be able to find any responsibilities for. They are called that way, because this is what they do: NOTHING. They just are.

But we do not speak corporation-bullshit (even though some of us work at corporations), we are Agile developers, we are writers, we name things as they are.

So our OrderService interface should be split into, let's say: OrdersImporter, OrdersExporter and OrdersReportFactory. That would make responsibilities and our intentions clear. If you do that, you end up with a WebServiceOrdersImporter implementation class, that has only 30 lines, 4 methods, only one of them public.

Now that's simple, isn't it?


Conclusion

It doesn't have to be expansive or difficult to make code better while you are reading it.

You shouldn't wait for the code to get really ugly. You shouldn't write TODOs only for obvious problems. Every time you have to read and understand some code, ask yourself a question: Can I make it easier? Can I make it more readable? Can I make it better?

Take at least a small step, every time you answer is yes. Good is better than good-enough. This helps turn the tide, change the (bio)degradation into positive evolution. In the end, your code gets better with time.

No one writes good code right from the start. Good code evolves from normal code, by using the Boy Scout Rule.

And if you cannot find any way to make the code better, at least eliminate blocks of code with methods. Stick to "7 lines per method", and "Single Responsibility of a method" rules.

May 9, 2010

Writer versus Constructor

What's the favorite toy from your childhood? For me, both my sisters and pretty much everyone I know that would be Lego. If you could only afford it (and in the times when I was a kid, that wasn't so easy in the post-soviet Poland), there was no greater fun, than the joy of unlimited freedom to create whatever you can only imagine. And the fact, that you had limited resources wasn't a real obstacle. It was a challenge.

Nearly all the programmers I know, fell in love with programming because of the same reason. To be able to put pure ideas to life, to create something meaningful out of nothing. Truth is, no other matter is so flexible, as the stuff we work with.

We are developers because we love to construct things that work. To give soul to the machine. To make it act on our behalf. It's like magic.

That's why it is so difficult for us to understand, that a real software craftsman is a writer, not a constructor.

Uncle Bob is talking about it at the beginning of “Clean Code”. When you are writing code, you are actually reading a lot more. How is that possible? Let's have a look at an example: you have a project going, and you take a new feature from the Scrum/Kanban wall of features. And you:
  1. read the code to find a place where putting some more code will give you the result you seek
  2. read the code to better understand the surroundings, to eliminate potential side effects of your changes
  3. write the test and the code that does what the feature is expected to do

You can start with an acceptance test first, but that only changes the order, not the fact, that the proportions of reading to writing, are always in favor of reading. Big time.

So, if reading takes so much time, how important is to make things readable?

People are working hard to learn how to write faster, how not to use mouse in their IDE, they memorize all the keyboard shortcuts, use templates and so on, but the speed up is not significant in real work. Why? Because we are only writing code 1/10 of our time. The rest is spent on reading and thinking.

A good craftsman is a good writer, because he makes the code readable, easy to grasp, easy to understand. There is a huge shift in mentality, between a constructor and a writer. Above all, the constructor's goal is to make things work. The writer's goal, on the other hand, is to make things easy to understand.

The question is, who are we writing code for? The computer? Hell no. If that was true, we would be still writing in an assembly language (or maybe even in a machine code). The whole point of procedural, functional, object oriented languages was to make it easier for us to understand the code. And that's because we don't write the code for the computer.

We write it for other programmers.

To make the shift in mentality easier, let mi show you the differences in mindsets, between the constructor and the writer.

The Constructor


The constructor is in love with technology for the sake of technology. He likes to “technologically masturbate” himself with new toys. He thinks that simplicity is for pussies, the real thing is always complex. He never removes anything, always adds more. He doesn't care that he cannot understand how his earlier solutions work, if they work why would he want to change them? He creates monsters like EJB1/2, that can do pretty much everything, but nobody is able to handle them. The he writes software to be able to use/confgure the software he has written before, because it's already too complex for anybody (maybe except for him) to understand how to use it directly.

A bigger framework is a solution to his every problem. And even if it's a new problem, he tries to modify his beloved one-to-rule-them-all tool, to be able to solve it. He is dreaming about the day, that he will have a single, big, multipurpose, futuristic wrench that works as if by magic, and does everything, from printing log files to saving the planet.

The constructor works best alone. He gets angry at people that cannot understand his programs in a second. When he works with other programmers, he is always afraid, he doesn't like others to play with his tools. When they try to do pretty much anything, they always blow something else up. Because of unforeseen side effects, a small change can devastate days of work. And it happens a lot. It gets messy, so he hates to work with others.

The Writer

The writer reads a lot, so he wants to make his code as readable, as simple as possible. He learns stuff like DSLs, patterns, eXtreme Programming, to make things easy. Easy is his keyword. He cares less about frameworks. He usually knows a few languages from different worlds, because he believes there is no single silver bullet, and different solutions are better for different problems. His code has few comments, because it doesn't need them. His code is self-documenting. His code is expressive. His code is simple, simplicity is his main value. Brilliant things are simple.

The writer always thinks about his reader. He walks hand to hand with his readers. He guides them, he cares for them. He never leaves them in the dark. His skills are in communication: he wants his intentions to be understood.

The writer works like a sniper. His changes (whether bug fixes or new features) are minimal, but the effects are exactly as expected. He never does Shotgun Surgery. He doesn't violate DRY. He is a precise man and he works with a scalpel. His code is side-effect free, simple and readable, so he knows where and how to hit. He knows what the effects will be. He keeps accidental complexity low.

The writer is agile. He doesn't set up traps, and he doesn't fall into traps. He works hard to master basic skills. He believes that without those basic skills, no framework can save him anyway.

The writer likes to work with other programmers. He is using pair programming and code reviews to make sure that his code is easy to understand by others. He wants to have another mind watching his back, and catching him when he falls. He is supportive, he is friendly. He understands that no man is an island, and that the quality of his work will be measured by the number of WTFs shouted by other developers.

A good craftsman is a good writer.

And you know what one of the most successful writers of recent times, Stephen King, says?
That he's only a craftsman.

PS: all the pictures are from Fallout games.
PS2: I've failed at giving appropriate credits to the author of the concept of programmer as a constructor. Let me fix that right away: the source of the idea that a programmer as a writer has opposition in programmer-constructor comes from a post by Andrzej Szczodrak that you can find in here