Thursday, February 25, 2010

It's possible to change it ... Code ownership

Another day, another story...
I stumble upon one piece of code, that caused an NPE.
It could have obviously been avoided doing a small refactoring so that any client of the code, no matter what, would not get an NPE. Doing so would have meant to delete one method, and possibly break the compilation of any client calling that code, but instead of a runtime NPE a compilation error is way better.

So I asked around, why it was not refactored :
"You can change it, but it will be your responsibility if it make fails other projects"

Nice.
If you are not too scared of being held responsible of another project delay, this would stay in the same state, and the NPE was normally fixed so why bother ?
Or you can do it like in cowboy style ( also known as Rambo style) doing all the work by yourself.

Both cases are not ideal, and the cause was :
  • there were no true Continuous Integration and appropriate testing strategy. As a consequence every one fear the change.
  • There is a sense of code ownership or I would even say worst : a disownership of the code :" It's not my code"
Because the next step of the Strong Code Ownership  , is the No OwnerShip at all.

The only way to go is obviously the Collective Code Ownership. Doing Pair Programming helps to get that sense, and Code Review, is another way if Pair Programming is not possible.

And then the "Knowledge Transfer" when someone leave the team becomes obsolete and useless.

Knowledge Transfer word is a Team Smell to see that they are not working in an "Agile"  way.

Wednesday, February 24, 2010

Semantics of Collections

What is up with the java.util.List?
I see it so frequently used, and most of the times to return collection of elements that are not ordered.

The true semantic of the List is  an ordered Collection
Most of the case the API returns unordered elements that are not duplicated. It is then the true semantic of a  java.util.Set.


If the API returns an unordered, collection of possibly duplicated elements, then the returned type should be a Bag (or Multiset from Google Collections) , and the semantic would be correct.

At least, in doubt I'd rather get a java.util.Collection than the wrong returned type.

Monday, February 22, 2010

Relection equals Matcher - Easy Mock made even easier

Easymock is great tool to produce mock object and do true unit testing verifying the expected behavior of your class.

One issue faced times to times is  while verifying the behavior it uses equals to verify objects passed as arguments are same.

It should work great in most of the cases, but living in the real world most of the times equals is not defined.
Of course you should define a true equals function, but sometimes you can't even do it, since you might not be able to modify the parameter.
Arguably you want to test the behavior of your class, no modifying the argument itself.

So here is a simple Matcher using Apache commons EqualsBuilder:
public  class ReflectionEqualsMatcher<t> implements IArgumentMatcher
{
    private T expected;

    public ReflectionEqualsMatcher(T expected)
    {
        this.expected = expected;
    }

    public static <S> S eqReflect(S toCompare)
    {
        EasyMock.reportMatcher(new ReflectionEqualsMatcher<S>(toCompare));
        return toCompare;
    }


    public boolean matches(Object o)
    {
        return EqualsBuilder.reflectionEquals(expected, o);
    }

    public void appendTo(StringBuffer buffer) {
        buffer.append("eqReflect(");
        buffer.append(expected.toString());
        buffer.append(")");

    }

Friday, February 19, 2010

Deprecated is bad in application code

Deprecated should be forbidden and never be used when coding an application.
The sole purpose of the Deprecated tag  ON AN API is

  • It is insecure, buggy, or highly inefficient
  • It is going away in a future release
  • It encourages bad coding practices
( from Java's  Deprecation definition)

When developing in-house software, when you have no outside client of your code it does not make sense at all.
Most of the time it is used when the developer is afraid to delete or modified the class/ method and therefore add a deprecated annotation instead of  refactoring.


It should be part of the code smells indicators along with commented code.

Monday, February 15, 2010

The multiplication of TODOs

Times to times, you must have seen in the existing code something like that

// TODO Auto-generated method stub
return null;

Great. Without knowing, you might call a method that is not implemented, and get a wonderful NPE.

I also like the

//TODO to fix later

Yeah right, it will be fixed.
Now the funny part. Let's go to the TODO tab in your IDE, and count the number of TODOs in that existing project: 890 for 320 files. An average of 3 TODOs per file. Wouhou !

Enough of ranting.

To avoid that, change your default file template, to instead of adding a TODO and return a null, throw a runtime Exception like:

throw new UnsupportedOperationException("This method is not implemented");

or even better use an explicit NotImplementedException:
throw new org.apache.commons.lang.NotImplementedException("TODO");
That is first step. Now the only way to enforce that the TO DO will BE DONE, is to create the / modify the test calling that piece of code. It will fail and force you to act.

Concerning the Fix Later example, I would recommand the use of a TODO to issue tracker converter, so that item would be part of the backlog, prioritized, and eventually done in some future iterations.

Friday, February 12, 2010

Commenting old code is useless

Why do people comment out old code ?

There is your favorite SCM for tracking the changes that happened in the particular piece of code.
So why ?
Because people are afraid of the change, afraid of breaking something.
They comment out the old code, so that if something happened during the Big Testing Phase, they could revert.

Commented code means no code coverage, i.e. no unit test.
Bad.
Commented code should be added as one of the code quality indicator (or technical debt management) like Sonar, because it smells like something not right is going on.

Thursday, February 11, 2010

Some quotes on Unit Testing

From an old article of Heinz Kabutz

JSC: What are some of the big mistakes that even experienced Java developers make?

Kabutz: Not unit testing. At conferences, I ask, "How many of you have unit tests for your code?" Almost no one raises their hands -- and these are experienced professionals. While programmers start projects believing they should have unit tests, they often don't take the time to write them up front, and they end up spending time fixing bugs. We should always perform unit tests.

A good unit test will not only flush out problems before you get close to production, but it will help regression test your system as well. I see it over and over -- very few companies have the discipline to enforce the use of unit tests.

And a following interview of Joshua Bloch

JSC: Java Champion Dr. Heinz Kabutz finds that failure to unit test is a big problem among Java developers. He reports, "At conferences, I ask, 'How many of you have unit tests for your code?' Almost no one raises their hands -- and these are experienced professionals." Your reaction?

Bloch: I'm really sorry to hear it. Unit tests are essential! If you don't have them, you don't know whether your code works. Having a good set of unit tests gives you much more confidence that your code works in the first place and that you don't introduce bugs as you maintain it. Inevitably, you will introduce bugs, but your unit tests will often let you find the bugs as soon as you introduce them, so you can fix them before they cause any damage.

Arggh, this was 2007.

3 years later I still see project /organization that does not believe in unit tests, and where I hear " We don't have the bandwidth to do unit test".

Prehistoric software development. Upfront tests, TDD, anyone ?

Wednesday, February 10, 2010

Sans maîtrise, la puissance n'est rien

Pardon my French : "Power is nothing without control" was a famous commercial catch phrase from a tire company.

It made me think of a project and agility methodologies.
How is that one translated in the IT world :
"Latest technologies is nothing without control of the technical debt."
Not very catchy but so true.

One example:
On this new project, designed to replace legacy systems, you take the latest technology stack, add some very knowledgeable developers.
But without using any XP principle or any dogmatic approach: no true unit tests, no true continuous integration , no knowledge sharing but in big lengthy meetings ( of course no pair programming or code review , uh what is that?).

Shake it and let it marinated for 2 years. Open it and you'll smell a very strong technical debt.

Nothing can be changed, because of the fear of breaking something .
Not mentioning the lost knowledge of why it was designed like that.

Forecast result : the developers play the job security becoming all Bob the developer's types, and your new top notch application is now really a mammoth application.

Back to Square one. Same player play again.