22 March 2014

Refactor, don't reinvent the wheel

Recently I saw a training regarding clean code and refactoring. One of shown examples of bad code was something like this:
public void setName(String name) {
    this.name = name;
    if (this.name != null ) {
        if (this.name.length() > 30) {
            this.name = name.substring(0,30);
            this.name = this.name.toUpperCase();
        } else {
            this.name = this.name.toUpperCase();
        }
    }
}
And after a few slides the code was refactored to the final version:
public void setName(String name) {
    if (!isValid(name)) {
        this.name = null;
    }

    this.name = limit(name, to(30)).toUpperCase();
}

private boolean isValid(String name) {
    return name != null;
}

private String limit(String input, int limit) {
    return input.substring(0, limit);
}

private int to(int x) {
    return x;
}
And I will argue that's a very wrong approach or a really bad example. Why? Because every language has its idioms and most commonly used tools that became de-facto standards and are well known and understood. In java world it's guava, apache commons, lambdaj etc. Using those libraries, you can be much more functional, null-safe and concise. You can use well known existing functions instead of creating new ones and learn them again in each project. In my opinion, much more readable would be:
public void setName(@Nullable String name) {
    this.name = StringUtils.upperCase( StringUtils.left(name, 30));
}
or in case we'll need it more than once:
public static upperCasePrefix(@Nullable String input, int limit) {
    return = StringUtils.upperCase( StringUtils.left(input, limit));
}

public void setName(@Nullable String name) {
    this.name = upperCasePrefix(name, 30);
}

1 komentarze :

halliearles said...
This comment has been removed by a blog administrator.

Post a Comment