Add flair to your code: Code Squiggles

Introduction to Code Squiggles, a coding style that improves the readability of your java code.

Wrought Iron by quadriremeFor several months now, I’m experimenting with a programming style that you might want to call “syntax aware programming”. Every coding step starts with the question “what do I want to achieve and how do I want to write it down?”. Then I proceed to write it down in this perfect manner, mostly some english sentence, and try to incrementally convert the syntax into something the java compiler stops complaining about. There’s a lot to be learnt about API design, naming and creative syntax usage this way. One thing I’ve discovered along the way are Code Squiggles.

Introduction to Code Squiggles

Code squiggles are little methods that contain no functionality, other than directly returning the single given parameter. Their purpose is to make the code more fluently readable by casual readers. Calling these methods is absolutely optional, as they offer no behaviour at runtime. But by bloating your code with these method calls, you lower the amount of syntax rules and conventions the reader has to know before being able to understand the code. The process of adding the method calls feels like adding “happy little squiggles” (I certainly miss Bob Ross!) to your code.

Adding Code Squiggles by example

Let me give you a full example how Code Squiggles can turn your boring old java code into something even non-programming readers can grasp without problems. Note that the process of transforming the code isn’t the process I initially described, but the way to deal with existing code.

The initial code fragment looks like this:

assertTrue(filesDirectory.getChild("0.png").isFile());

As you can see, it’s an assertion line of an unit test. We improve the readability by extracting the intent of the assertion into the called method name (it’s a normal “extract method” refactoring):

assertFileExists("0.png", filesDirectory);

Now is the time to add the first Code Squiggle:

assertFileExists("0.png", in(filesDirectory));

You’ve noticed the difference? It’s just the word “in”, but it improves the semantics of the second parameter. As I promised, Code Squiggles are methods without any functionality worth talking about:

protected VirtualFile in(VirtualFile filesDirectory) {
    return filesDirectory;
}

The method takes a parameter and returns it. The real “functionality” of this method lies within its name. Everything else is only necessary to overcome (or overcode) java’s shortcoming of advanced syntax definition measures.

That’s all about Code Squiggles. But it doesn’t stop here. Let’s improve the example to its final shape, when we add two squiggles:

assertFile(withName("0.png"), existsIn(filesDirectory));

Read this line out loud! And notice the subtle change in the assertion method’s name. It begins to deminish clarity without the Code Squiggles. That’s when your API begins to depend on them. But it’s still perfectly valid java code if you just omit them.

The conceptual origin of Code Squiggles clearly comes from the behaviour driven development (BDD) style of writing tests and the ScalaTest Matchers. So I can’t claim much originality or cleverness myself here. But Code Squiggles, despite this initial example, aren’t limited to test code.

Adding inline documentation with Code Squiggles

Remember the last time you needed to integrate an horrible third-party API? Probably, there was a method with seven or eight parameters and all of them were primitive ints. No matter how often you called this method, you couldn’t remember the order of these ints. That’s when Code Squiggles might help you a bit.

This is the signature of a fairly complex method:

protected int performCalculation(int value, int lowerLimit, int upperLimit, int offset) { ...

Therefore, your call isn’t very self-explanatory:

int result = performCalculation(10, 2, 23, 13);

But it might be a lot more understandable when you add some squiggles:

int result = performCalculation(value(10), lowerLimit(2), upperLimit(23), offset(13));

I won’t spell out the squiggle implementations, as they should be straight-forward. Note that the java compiler doesn’t catch up here. You can easily swap the squiggles around to obfuscate your code. All you got is read-time safety. If you want compile-time safety, you need to replace the primitives with types and probably pay for some rounds of beer for the third-party API developers to include your changes or write an adapter (“corruption layer” is my preferred term here).

Regular use of Code Squiggles

If you want to use squiggles a lot, you might think about collecting them in a dedicated class. Design them to be static and generic, and you can use them easily with static imports:

public static <T> T existsIn(T instance) {
    return instance;
}

But remember that there are reserved keywords in java that might hamper you a bit.

Conclusion

Code Squiggles are useless bloat to your code unless you value read-time safety or casual readability. They can tidy up your code to a point where method or even class names are affected to form a block of code where you only have to replace the funnier chars with blanks to obtain a paragraph of plain written english anybody can read and understand.

What are your ideas towards Code Squiggles? Have you used them on your code? Mind to share the result?

Code squiggles are little methods that contain no functionality, other than directly returning the single given parameter. Their purpose is to make the code more fluently readable by casual readers.

The C++ Shoot-yourself-in-the-Foot of the Week

I think we can all agree that C++, compared to other languages, provides quite a number of possibilities to screw up. Everybody working with the language at some point probably had problems with e.g. its exception system, operator overloading or automatic type conversions – to name just a few of the darker corners.

There are also many mitigation strategies which all come down to ban certain aspects of the language and/or define strict code conventions. If you follow Google’s style guide, for example, you cannot use exceptions and you are restricted to a very small set of boost libs.

But developers – being humans – often find creative and surprising ways to thwart every good intentions. In an external project the following conventions are in place:

  • Use const wherever you possibly can
  • Use boost::shared_ptr wherever it makes sense.
  • Define typedefs to shared_ptrs  in order to make code more readable.
  • typedefs to shared_ptrs are to be defined like this:
typedef boost::shared_ptr&lt;MySuperDuperClass&gt; MySuperDuperClassPtr;
  • typedefs to shared const pointers are to be defined like this:
typedef boost::shared_ptr&lt;const MySuperDuperClass&gt; MySuperDuperClassCPtr;

As you can see, postfixes Ptr and CPtr are the markers for normal shared_ptrs and constant shared_ptrs.

Last week, a compile error about some const to non-const conversion made me nearly pull my hair out. The types of variables that were involved all had the CPtr postfix but still the code didn’t compile. After a little digging I found that one of the typedefs involved was like this:

typedef boost::shared_ptr&lt;  MySuperDuperClass&gt; MySuperDuperClassCPtr;

Somebody just deleted the const modifier in front of MySuperDuperClass but left the name with the CPtr untouched. And because non-const to const conversions are allowed this was not detected for a couple of weeks. Nice going!

Any suggestions for a decent style checker for c++? Thanks in advance 😉

Wrestling with Qt’s Model/View API – Filtering in Tree Models

Qt4’s model/view API can be kind of a challenge sometimes. Well, prepare for a even harder fight when sorting and filtering come into play.

As I described in one of my last posts, Qt4’s model/view API can be kind of a challenge sometimes. Well, prepare for a even harder fight when sorting and filtering come into play.

Let’s say you finally managed to get the data into your model and to provide correct implementations of the required methods in order for the attached views to display it properly. One of your next assignments after that is very likely something like implementing some kind of sorting and filtering of the model data. Qt provides a simple-at-first-sight proxy architecture for this with API class QSortFilterProxyModel as main ingredient.

Small preliminary side note: Last time I checked it was good OO practice to have only one responsibility for a given class. And wasn’t that even more important for good API design? Well, let’s not distract us with such minor details.

With my model implementation, none of the standard filtering mechanisms, like setting a filter regexp, were applicable, so I had to override method

QVariant filterAcceptsRow ( int source_row, const QModelIndex& sourceParent ) const

in order to make it work. Well, the rows disappeared as they should, but unfortunately so did all the columns except the first one. So what to do now? One small part of the documentation of QSortFilterProxyModel made me a little uneasy:

“… This simple proxy mechanism may need to be overridden for source models with more complex behavior; for example, if the source model provides a custom hasChildren() implementation you should also provide one in the proxy model.”

What on earth should I do with that? “… may need to be overridden“? “… for example.. hasChildren()…” Why can’t they just say clearly what methods must be overridden in which cases???

After a lot more trial and error I found that for whatever reason,

int columnCount ( const QModelIndex& parent ) const

had to be overridden in order for the columns to reappear. The implementation looks like what I had thought the proxy model would do already:

int MyFilter::columnCount ( const QModelIndex& parent ) const
{
   return sourceModel()->columnCount(parent);
}

So beware of QSortFilterProxyModel! It’s not as easy to use as it looks, and with that kind of fuzzy documentation it is even harder.

About PrintStream and Exceptions

Several of our projects deal with sensor hardware of different types often connected via the good old™ serial port. That is fine most of the time because most protocols are simple and RXTX provides a nice cross-platform library for most of your serial port needs. But many new computers do not feature the old RS232 serial ports anymore or other contraints prevent the use of a plain RS232 serial port. Here come serial converters like the Advantech ADAM 4570 (serial-to ethernet) or usb-to-serial converters into play. Usually this works fine.

Now one of our customers had a test system using an unreliable converter with sensor hardware. The hardware problems uncovered a robustness issue in our software which crashed the JVM when the virtual serial port of the converter disappeared and our app tried to write to it. Despite the faulty hardware our software had to be robust because it manages many more devices other than just that one sensor over serial. Looking at the problem we discovered that the crash occurred somewhere in the native part of RXTX. So we decided to scratch our own itch (and the one of the customer) and set out to fix the issue in RXTX at a Open Source Love Day (OSLD) . So we fixed the problem and submitted the patch to the bugtracker of the RXTX project. Our sample program now worked flawlessly and threw an IOException when the serial port failed in some way.

Happy to have fixed the problem we incorporated the patch RXTX in our production software but it still crashed and no IOException appeared anywhere in the logs. After another bughunting session we spotted the subtle difference of sample and production program: the use of OutputStream insted of PrintStream. PrintStream silently swallows all exceptions which proved fatal in our use case with the unreliable stream carrier. So the final fix was essentially replacing our PrintStream code

RXTXPort port = new RXTXPort("COM6");
PrintStream p = new PrintStream(port.getOutputStream(), true, "iso8859");
p.print("command");

with using OutputStream directly:

RXTXPort port = new RXTXPort("COM6");
OutputStream o = port.getOutputStream();
o.write("command".getBytes("iso8859"));

Conclusion

Be careful when using PrintStream with unreliable stream carriers it swallows exceptions! That may shadow problems which you may want to know of. Often PrintStreams behaviour will not be a problem but in certain cases like the one depicted above it causes a lot of headaches.

Find the bug: Groovy and autogenerated equals

Every program has bugs. Even autogenerated code from IDEs. Can you spot this one?

Take this simple Groovy class:

public class TestClass {
    String s
}

Hitting ‘autogenerate equals and hashCode’ a popular IDE generates the following code:

public class TestClass {

    String s


    boolean equals(o) {
        if (this.is(o)) return true;

        if (!o || getClass() != o.class) return false;

        TestClass testClass = (TestClass) o;

        if (s ? !s.equals(testClass.s) : testClass.s != null) return false;

        return true;
    }

    int hashCode() {
        return (s ? s.hashCode() : 0);
    }
}

At first this looks unusual but ok, but taking a closer look there is a bug in there, can you spot it? And even better: write a test for it?

Lessons learned:

  • If something looks suspicious write a test for it.
  • Question your assumptions. There is no assumption which is safe from be in question.
  • Don’t try to be too clever.

Prettier failures using Swing TaskDialog

An introduction to the Swing TaskDialog project, a fine little gem to spice up your (java swing) dialogs. Includes a real usage example.

The standard way to present graphical user interfaces (GUI) on a desktop machine in java is to use Swing. It’s a very flexible API with a steep learning curve and some oddities (e.g. EDT handling is cumbersome at least), beginning to show some age. There were several attempts to take the Swing experience to a new level, including the marvellous book “Filthy Rich Clients” by Chet Haase (we miss you in the Java camp!) and Romain Guy. So Swing isn’t dead or dying, it’s just getting old.

A pain point of Swing

One thing always bothered me with Swing: It is relatively easy to present a basic message or input dialog. But to add slightly more complexity to a dialog suddenly means substantially more effort. Dialogs don’t scale in Swing. If you ever “designed” an error dialog for your end user, presenting the essence of an exception that just occurred, you already know what I’m talking about. I have to make a confession: Our exception/error dialogs were nearly as nasty as the exception itself. But nobody wants to fail nasty.

Swing TaskDialog to the rescue

At late february this year, Eugene Ryzhikov published his Swing TaskDialog project on his blog. His release pace has been a new version once a week since then. So I’m writing on a moving target.

The TaskDialog project provides basic message, progress and input dialogs based on the operating system’s “User Experience Guidelines”. The visual content is very appealing as a result. But the project doesn’t stop here. The programming API is very understandable and to the point. You don’t have to hassle with big concepts to use it, just look at the examples and start from there.

It was a matter of minutes to replace our old, nasty error dialog with a much prettier one using TaskDialog. Here are two screenshots of it in action, with the detail section retracted (initial state) and flipped open.

Of course, this is only the Windows version of the dialog. You should head over to the TaskDialog examples page to get an idea how this might look on a Mac. This is a dialog that’s pretty enough to not scare the user away by sheer uglyness. The code for this dialog is something like:


TaskDialog dialog = new TaskDialog("Error during process execution");
 dialog.setIcon(TaskDialog.StandardIcon.ERROR);
 dialog.setInstruction("An error occurred during the execution of process 'DemoProcess':");

 Exception exception = new Exception("Because it's just a demo");
 StringBuilder detailMessage = new StringBuilder();
 for (StackTraceElement stackTraceElement : exception.getStackTrace()) {
 detailMessage.append(stackTraceElement.toString());
 detailMessage.append("\n");
 }
 dialog.setText("Error message: <b>" + exception.getMessage() + "</b>\n\n<i>This incident was traced and logged.</i>");
 dialog.getDetails().setExpandableComponent(
 new JLabel(Strings.toHtml(detailMessage.toString())));
 dialog.getDetails().setExpanded(false);

 JLabel waitLabel = new JLabel(Strings.toHtml("<i>This dialog closes automatically in 26s</i>"));
 dialog.setFixedComponent(waitLabel);

 dialog.show();

Notice the usage of Strings.toHtml() to convert plain Strings to HTML-rendered rich text elements.

Timed dialogs

If you look at the presented information, you’ll notice it’s just a demo presenting a fake exception. But you’ll notice another thing, too: This dialog is about to close itself automatically soon. This is a speciality of our project: The GUI runs unattended by users for long periods of time. If you encounter an error every ten minutes and an user returns to the screen after a week, the system isn’t accessable without closing a million dialogs first. You might argue why a system error lasts for a week, but that’s a reality in this project we cannot change. So we came up with timed dialogs that go away on their own after a while. The information of the dialog is persisted in the log files that get evaluated periodically.

The TaskDialog API provides easy integration for a GUI widget to be included in the dialog. In our timed dialog use case, it’s a JLabel, as highlighted in the code example at lines 16 and 17. A background thread periodically updates the text and closes the dialog when time runs out. But you’ll find examples with progress bars and other components on Eugene’s blog.

Conclusion

The Swing TaskDialog project is a fine little gem to spice up your application. It’s API is simple, yet powerful and has proven customizable to our special use case. Finally, effort for basic dialogs in Swing scales again.

Follow-up to our Dev Brunch March 2010

A follow-up to our March 2010 Dev Brunch, summarizing the talks and providing bonus material.

Yesterday we held our Dev Brunch for this month. It was the second brunch in our new office, with some attendees visiting it for the first time. The reactions were the same: “I want to move in here!”. The topics were of different kinds, from live presentations to mere questions open for discussion.

The Dev Brunch

If you want to know more about the meaning of the term “Dev Brunch” or how we implement it, have a look at the follow-up posting of the brunch in October 2009. We continued to allow presence over topics. These topics were discussed today:

  • Singleton vs. Monostate – We all know that Singletons are bad for your test coverage, they make a poor performance on your dependency chart and are generally seen as “evil”. We discussed the Monostate pattern and if it could solve some of the problems Singletons inherently bring along. Based upon the article from Uncle Bob, we concluded that Monostates are difficile at least and don’t help with the abovementioned problems.
  • What is “agile” for you? – This simple question provoked a lot of thoughts. You can always obey the Agile Manifesto word by word without understanding what the deeper motives are. The answer that fitted best was: “You can name it when you see it”. We concluded that it’s easy and common practice to label any given process “agile” just to sound modern.
  • News around Yoxos – If you are using Eclipse, you’ve certainly heard about Yoxos already. Now during the EclipseCon 2010, good news were announced. We got a sneak peek on the new Yoxos Launcher and how it will help in managing your pack of Eclipse installations. We are looking forward to become beta testers because we can’t wait to use it.
  • Teaser talk for “Actors in Scala” – The actor paradigm for parallel programming is a promising alternative to threads. While threads are inevitable complex even for simple tasks, actors seem to recreate  a more natural approach to parallelism. This talk was only the teaser for a more in-depth talk next time, with hands-on code examples.
  • Properties in Scala – This talk had lots of code examples and hands-on discussion about the Properties feature of Scala. Properties are an elegant way to reduce your boilerplate code for simple objects and to sustain compatibility with Java frameworks that rely on the Java Beans semantics. We clearly understood the advantages, but ran into some strangeness related to the conjoint namespaces of fields and methods along the way. Scala isn’t Java, that’s for sure.
  • Introduction to PreziPrezi is a modern presentation tool in the tradition of the dreaded PowerPoint or Apple’s Keynote. It adds a twist to your presentation by adding two new dimensions: laying out everything on a big single canvas (no slides!) and relying heavily on zooming effects. The online editor is surprisingly usable, yet simple and lightweight. If you want to meet prezi, check out the introduction prezis and the showcase on their homepage.

As usual, the topics ranged from first-hand experiences to literature research. For additional information, check out the comment sections. Comments and resources might be in german language.

Retrospection of the brunch

We keep getting better in timing our talks. We nearly maintained our time limit and didn’t hurry anything. For the next brunch, we are looking forward to use our new office roof garden to brunch and talk in the springtime sun.

Verbosity is not Java’s fault

One of Java’s most heard flaws (verbosity) isn’t really tied to the language it is rooted in a more deeply source: it comes from the way we use it.

Quiz: Whats one of the most heard flaws of Java compared to other languages?

Bad Performance? That’s a long overhauled myth. Slow startup? OK, this can be improved… It’s verbosity, right? Right but wrong. Yes, it is one of the most mentioned flaws but is it really inherit to the language Java? Do you really think Closures, annotations or any other new introduced language feature will significantly reduce the clutter? Don’t get me wrong here: closures are a mighty construct and I like them a lot. But the source of the problem lies elsewhere: the APIs. What?! You will tell me Java has some great libraries. These are the ones that let Java stand out! I don’t talk about the functionality of the libraries here I mean the design of the API. Let me elaborate on this.

Example 1: HTML parsing/manipulation

Say you want to parse a HTML page and remove all infoboxes and add your link to a blog box:

        DOMFragmentParser parser = new DOMFragmentParser();
        parser.setFeature("http://xml.org/sax/features/namespaces", false); 
        parser.setFeature("http://cyberneko.org/html/features/balance-tags", false);
        parser.setFeature("http://cyberneko.org/html/features/balance-tags/document-fragment", true);
        parser.setFeature("http://cyberneko.org/html/features/scanner/ignore-specified-charset", true);
        parser.setFeature("http://cyberneko.org/html/features/balance-tags/ignore-outside-content", true);
        HTMLDocument document = new HTMLDocumentImpl();
        DocumentFragment fragment = document.createDocumentFragment();
        parser.parse(new InputSource(new StringReader(html)), fragment);
        XPathFactory factory = XPathFactory.newInstance();
        XPath xpath = factory.newXPath();
        Node infobox = xpath.evaluate("//*/div[@class='infobox']", fragment, XPathConstants.NODE);
        infobox.getParentNode().removeChild(infobox);
        Node blog = xpath.evaluate("//*[@id='blog']", fragment, XPathConstants.NODE);
        NodeList children = blog.getChildNodes();
        for (int i = 0; i < children.getLength(); i++) {
            node.remove(children.item(i));
        }
        blog.appendChild(/*create Elementtree*/);

What you really want to say is:

HTMLDocument document = new HTMLDocument(url);
document.at("//*/div[@class='infobox']").remove();
document.at("//*[@id='blog']").setInnerHtml("<a href='blog_url'>Blog</a>");

Much more concise, easy to read and it communicates its purpose clearly. The functionality is the same but what you need to do is vastly different.

  The library behind the API should do the heavy lifting not the API's user.

Example 2: HTTP requests

Take this example of sending a post request to an URL:

HttpClient client = new HttpClient();
PostMethod post = new PostMethod(url);
for (Entry param : params.entrySet()) {
    post.setParameter(param.key, param.value);
}
try {
    return client.executeMethod(post);
} finally {
    post.releaseConnection();
}

and compare it with:

HttpClient client = new HttpClient();
client.post(url, params);

Yes, there are cases where you want to specify additional attributes or options but mostly you just want to send some params to an URL. This is the default functionality you want to use, so why not:

  Make the easy and most used cases easy,
    the difficult ones not impossible to achieve.

Example 3: Swing’s JTable

So what happens when you designed for one purpose but people usually use it for another one?
The following code displays a JTable filled with attachments showing their name and additional actions:
(Disclaimer: this one makes heavy use of our internal frameworks)

        JTable attachmentTable = new JTable();
        TableColumnBinder<FileAttachment> tableBinding = new TableColumnBinder<FileAttachment>();
        tableBinding.addColumnBinding(new StringColumnBinding<FileAttachment>("Attachments") {
            @Override
            public String getValueFor(FileAttachment element, int row) {
                return element.getName();
            }
        });
        tableBinding.addColumnBinding(new ActionColumnBinding<FileAttachment>("Actions") {
            @Override
            public IAction<?, ?>[] getValueFor(FileAttachment element, int row) {
                return getActionsFor(element);
            }
        });
        tableBinding.bindTo(attachmentTable, this.attachments);

Now think about you had to implement this using bare Swing. You need to create a TableModel which is unfortunately based on row and column indexes instead of elements, you need to write your own renderers and editors, not talking about the different listeners which need to map the passed indexes to the corresponding element.
JTable was designed as a spreadsheet like grid but most of the time people use it as a list of items. This change in use case needs a change in the API. Now indexes are not a good reference method for a cell, you want a list of elements and a column property. When the usage pattern changes you can write a new library or component or you can:

  Evolve your API.

Designed to be used

So why is one API design better than another? The better ones are designed to be used. They have a clearly defined purpose: to get the task done in a simple way. Just that. They don’t want to satisfy a standard or a specification. They don’t need to open up a huge new world of configuration options or preference tweaks.

Call to action

So I argue that to make Java (or your language of choice) a better language and environment we have to design better APIs. Better designed APIs help an environment more than just another new language feature. Don’t jump on the next super duper language band wagon because it has X or Y or any other cool language feature. Improve your (API) design skills! It will help you in every language/environment you use and will use. Learning new languages is good to give you new viewpoints but don’t just flee to them.

FindBugs-driven bughunting in legacy projects

I have been working on a >100k lines legacy project for a while now. We have to juggle customer requests, bug fixes and refactoring so it is hard to improve the quality and employ new techniques or tools while keeping the software running and the clients happy. Initially there were no unit tests and most of the code had a gigantic cyclomatic complexity. Over the course of time we managed to put the system under continuous integration, employed quite some unit tests and analyzed code “hotspots” and our progress with crap4j.

Normally we get bug reports from our userbase or have to test manually to find bugs. A few weeks ago I tried a new approach to bughunting in legacy projects using FindBugs. Many of you surely know this useful tool, so I just want to describe my experiences in that project using FindBugs. Many of the bugs may be in parts of the application which are seldom used or only appear in hard to reproduce circumstances. First a short list of what I encountered and how I dealt with it.

Interesting found bugs in the project

  • There was a calculation using an integer division but returning a double. So the actual computation result was wrong but yet the error would have been hard to catch because people rarely recalculate results of a computer. When writing the test associated to the bugfix I found a StackOverFlowError too!
  • There were quite some null dereferences found, often in contructs like
     if (s == null && s.length() == 0)
     

    instead of

    if (s == null || s.length() == 0)
    

    which could be simplified or rewritten anyway. Sometimes there were possibilities for null dereferences on some paths despite of several null checks in the code.

  • Many performance bugs which may or may not have an effect on overall performance of the system like: new String(), new Integer(12), string concatenation across loops, inefficient usage of java.util.Map.keySet() instead of java.util.Map.entrySet() etc.
  • Some dead stores of local variables and statements without effect which could be thrown away or be corrected to do the intended things.

Things you may want to ignore

There are of course some bugs that you may ignore for now because you know that it is a common pattern in the team and abuse and thus errors are extremely unlikely. I, for example, opted to ignore some dozens of “may expose internal representation” found bugs regarding arrays in interfaces or accessibly via getters because it is a common pattern on the team not to tamper existing arrays as they are seen as immutable by the team members. It would have taken too much time to fix all those without that much of a benefit.

You may opt to ignore the performance bugs too but they are usually easy to fix.

Tips

  • If you have many foundbugs, fix the easy ones to be able to see the important ones more easily.
  • Ignore certain bug categories for now, fix them later, when you stumble upon them.
  • Concentrate on the ones that lead to wrong behaviour and crashes of your application.
  • Try to reproduce the problem with unit test and then fix the code whenever feasible! Tests are great to expose the bug and fix it without unwanted regressions!
  • Many bugs appear in places which need refactoring anyway so here is your chance to catch several flies at once.

Conclusion

With FindBugs you can find common programming errors sprinkled across the whole application in places where you probably would not have looked for years. It can help you to understand some common patterns of your team members and help you all to improve your code quality. Sometimes it even finds some hard to spot errors like the integer computation or null dereferences on certain paths. This is even more true in entangled legacy projects without proper test coverage.

A more elegant way to equals in Java

Implementing equals and hashCode in Java is a basic part of your toolbox. Here I describe a cleaner and less error-prone way to use in your code.

— Disclaimer: I know this is pretty basic stuff but many, many programmers are doing it still wrong —
As a Java programmer you know how to implement equals and that hashCode has to be implemented as well. You use your favorite IDE to generate the necessary code, use common wisdom to help you code it by hand or use annotations. But there is a fourth way: introducing EqualsBuilder (not the apache commons one which has some drawbacks over this one) which implements the general rules for equals and hashCode:

public class EqualsBuilder {

  public static interface IComparable {
      public Object[] getValuesToCompare();
  }

  private EqualsBuilder() {
    super();
  }

  public static int getHashCode(IComparable one) {
    if (null == one) {
      return 0;
    }
    final int prime = 31;
    int result = 1;
    for (Object o : one.getValuesToCompare()) {
      result = prime * result
                + EqualsBuilder.calculateHashCode(o);
    }
    return result;
  }

  private static int calculateHashCode(Object o) {
    if (null == o) {
      return 0;
    }
    return o.hashCode();
  }

  public static boolean isEqual(IComparable one,
                                              Object two) {
    if (null == one || null == two) {
      return false;
    }
    if (one.getClass() != two.getClass()) {
      return false;
    }
    return compareTwoArrays(one.getValuesToCompare(),
              ((IComparable) two).getValuesToCompare());
  }

  private static boolean compareTwoArrays(Object arrayOne, Object arrayTwo) {
      if (Array.getLength(arrayOne) != Array.getLength(arrayTwo)) {
        return false;
      }
      for (int i = 0; i < Array.getLength(arrayOne); i++) {
        if (!EqualsBuilder.areEqual(Array.get(arrayOne, i), Array.get(arrayTwo, i))) {
          return false;
        }
      }
      return true;
  }

  private static boolean areEqual(Object objectOne, Object objectTwo) {
    if (null == objectOne) {
      return null == objectTwo;
    }
    if (null == objectTwo) {
      return false;
    }
    if (objectOne.getClass().isArray() && objectTwo.getClass().isArray()) {
        return compareTwoArrays(objectOne, objectTwo);
    }
    return objectOne.equals(objectTwo);
  }

}

The interface IComparable ensures that equals and hashCode are based on the same instance variables.
To use it your class needs to implement the interface and call the appropiate methods from EqualsBuilder:

public class MyClass implements IComparable {
  private int count;
  private String name;

  public Object[] getValuesToCompare() {
    return new Object[] {Integer.valueOf(count), name};
  }

  @Override
  public int hashCode() {
    return EqualsBuilder.getHashCode(this);
  }

  @Override
  public boolean equals(Object obj) {
    return EqualsBuilder.isEqual(this, obj);
  }
} 

Update: If you want to use isEqual directly one test should be added to the start:

  if (one == two) {
    return true;
  }

Thanks to Nyarla for this hint.

Update 2: Thanks to a hint by Alex I fixed a bug in areEqual: when an array (especially a primitive one) is passed than the equals would return a wrong result.

Update 3: The newly added compareTwoArrays method had a bug: it resulted in true if arrayTwo is bigger than arrayOne but starts the same. Thanks to Thierry for pointing that out.