The danger of a poorly written equals in Java

Recently, I was refactoring a system at work and ran across a rather hard-to-track down bug after writing a unit test.  The data structure was an ordered list of what were essential ordered 3-tuples.  At first, the goal of the refactoring was to move classes into new packages then verify that nothing was broken.  Along the way of doing the testing I was alerted to the fact that the default set in the ordered set had changed and that needed to be updated as well and this is where the problem reared it’s ugly head.

I made the changes to the default set to mimic those on the server and then when I compared the new set to the set on the server, I was coming up short on the number of tuples.  Doing a diff of the rules showed two cases:

  1. Incorrect tuples
  2. Duplicate tuples
  3. Missing tuples

I quickly compared all the tuples I created to the tuples on the main list and verified that they were correct.  Once these were verified, I set a break point to see why tuples were missing.  As I stepped through the code I noticed that there was a point where the list stopped growing and items were being replaced when they shouldn’t have.  Obviously, this means we’ve got a problem with the equals.

Looking at the equals, the error was subtle, they attempted to use an != command on two Integer classes expecting it to compare the integer value of the class and not the hashcode.  I made the appropriate change to the equals method and re-ran the test and it passed without problems.

Of course, here I was able to write up what was basically an entire day of work in a few paragraphs… you’d think it’d be easy to spot a poorly written equals or even figure out that is the problem when you begin to see issues in production code.  However, the nature of production code and the complexities of the systems often lead you down rat holes in the search for the true problem.

Update on @Autowired considered harmful

After writing my post about the pox of @Autowired, I’ve been doing a lot of things with Spring and have softened my position a little bit.  Spring is a very powerful tool and, unfortunately, with anything that is powerful you need guidelines and constraints to keep from doing something stupid that will cause a lot of pain down the road.  Perhaps my hardline on @Autowired to not be used anywhere was a little short-sighted, however, I’m still weary of it in many cases (due to the experiences I’ve had using it).  Anyway, the new stance I have is that @Autowired should be used purely on constructors and kept away from injecting dependencies any other way.  As a wonderful side effect, this is allows you to hold true with Josh Bloch’s Effective Java rule to prefer immutability over mutability and fail fast.

@Autowired considered harmful

I guess I’m on an opinionated roll, after declaring my dislike of the Singleton design pattern, I’m taking a swing at @Autowired.

Spring is a great framework for application development for Java and even though it’s now arguably more complex than Java itself, it’s advantages far outweigh the problems.  The best part of it to me is the IoC container however, it has introduced one of the largest poxes (next to Singleton :)) to Java coders everywhere: @Autowired.  I’ve seen more problems occur in code due to the use of @Autowired and package scan.  In fact, Colin Yates of Expert Spring MVC + Webflow fame, has this to say about autowiring:

If you insist on sticking with autowiring, there are three different types of autowiring … bad, dangerous and terrible.

And I don’t think I can say it much better than that.

Singleton: A Design Pattern Past Its Time?

I recently had a conversation with someone and in passing I said something along the lines of singleton being a pointless design pattern and something that I’d try my best to not use. They said there were lots of reasons for a singleton, then we moved onto a different topic.  However, it’s gotten me thinking, is there really a reason for the use of a singleton?  I did some digging through StackOverflow and found a few reasons as to why to use one — but even then, it wasn’t a glowing endorsement.  Nothing I’ve seen has really made me change my mind from the fact that the singleton pattern is something with very limited usability and most people seem to view the singleton pattern as a pox on the programming world.

Personally, I’d stay as far as way as possible from them unless absolutely necessary.

What do y’all think?  Is it now an anti-pattern or is it something that is still useful?

Testing in Java: Part II – Writing a Fake

In the first part we talked about the domain objects and how to test a domain object.  Now it’s time to look at how to persist them to an in-memory data store for testing purposes.  We’ll be writing this in such a way that at a later date you can replace the in-memory representation with a strategy that persists the data to disk.  So, let’s jump right into the code starting with the data access object interface.

package com.laneholloway.unittestingexample.database.dao;

import com.laneholloway.unittestingexample.domain.MyItem;

import java.util.UUID;

/**
 * A simple Data Access Object
 */
public interface MyDao {
    public boolean exists(UUID guid);
    public MyItem get(UUID guid);
    public boolean save(MyItem item);
    public void delete(UUID guid);
}

Looking at the code you can easily tell that there are some simple CRUD operations and an exists command. Any datasource that we want to write the data to must implement these commands and inside the program, we should only use these commands to interact with the object. In order to facilitate the testing of the framework, I’ve created a simple in memory dao and placed it into the test directory that maven creates.

package com.laneholloway.unittestingexample.database.dao;

import com.laneholloway.unittestingexample.domain.MyItem;

import java.util.HashMap;
import java.util.Map;
import java.util.UUID;

/**
 * An in-memory representation of a datastore using MyDao
 */
public class MyInMemoryDao implements MyDao {
    private Map<UUID, MyItem> storedItems;

    public MyInMemoryDao() {
        this.storedItems = new HashMap<UUID, MyItem>();
    }

    @Override
    public boolean exists(UUID guid) {
        return this.storedItems.containsKey(guid);
    }

    @Override
    public MyItem get(UUID guid) {
        return this.storedItems.get(guid);
    }

    @Override
    public boolean save(MyItem item) {
        this.storedItems.put(item.getGuid(), item);
        return true;
    }

    @Override
    public void delete(UUID guid) {
        this.storedItems.remove(guid);
    }
}

The in memory dao uses a Map to hold the MyItem elements. As you can tell, the implementation is straight-forward and doesn’t pull any punches. It does exactly what it needs to do in order for us to test other parts of the system that require the use of the MyDao interface. In order for the other Maven modules to use this test implementation, we have to do a little work in Maven to expose this as a test class. In the POM file relating to the doa-interface module, we add this bit which creates a test jar file.

    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-jar-plugin</artifactId>
                <executions>
                    <execution>
                        <goals>
                            <goal>test-jar</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>
        </plugins>
    </build>

Now, using this test jar we can begin to test classes that use the MyDao interface. To use this test jar you modify the POM files for all modules that need access to this code with a dependency with it’s scope set to test.

In our case, we have the MyCommonInterface class which you’ll typically see in applications to give the consumers of the code a unified API to the CRUD operations abstracting away the DAO or DAOs that might make up the data storage of the application.

package com.laneholloway.unittestingexample.database.commoninterface;

import com.laneholloway.unittestingexample.domain.MyItem;

import java.util.UUID;

/**
 * A common interface to abstract away the various DAOs and allow the consumer of the API one interface to all
 * data.
 */
public interface MyCommonInterface {
    public boolean exists(UUID guid);
    public MyItem get(UUID guid);
    public boolean save(MyItem item);
    public void delete(UUID guid);
}

I’ve also supplied a simple implementation that uses a single DAO,

package com.laneholloway.unittestingexample.database.commoninterface;

import com.laneholloway.unittestingexample.domain.MyItem;
import com.laneholloway.unittestingexample.database.dao.MyDao;

import java.util.UUID;

/**
 * A very simple common interface
 */
public class MyCommonInterfaceForOneDao implements MyCommonInterface {
    private final MyDao dao;

    public MyCommonInterfaceForOneDao(MyDao myDao) {
        this.dao = myDao;
    }

    @Override
    public boolean exists(UUID guid) {
        return this.dao.exists(guid);
    }

    @Override
    public MyItem get(UUID guid) {
        return this.dao.get(guid);
    }

    @Override
    public boolean save(MyItem item) {
        return this.dao.save(item);
    }

    @Override
    public void delete(UUID guid) {
        this.dao.delete(guid);
    }
}

You can see that in the constructor, it expects a variable of type MyDao and then uses to perform CRUD operations. In order to properly test this, we need to use the in memory implementation so we don’t have to worry about creating a database, cleaning the database, and all the other database tasks needed just to test this interface. Using the this implementation, we can create a JUnit test class like this:

package com.laneholloway.unittestingexample.database.commoninterface;

import com.laneholloway.unittestingexample.database.dao.MyDao;
import com.laneholloway.unittestingexample.database.dao.MyInMemoryDao;
import com.laneholloway.unittestingexample.domain.MyItem;
import com.laneholloway.unittestingexample.domain.MyItemImpl;
import org.junit.Before;
import org.junit.Test;

import java.util.UUID;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class MyCommonInterfaceForOneDaoTest {
    private MyCommonInterfaceForOneDao myCommonInterfaceForOneDao;

    @Before
    public void setup() {
        MyDao myDao = new MyInMemoryDao();
        myCommonInterfaceForOneDao = new MyCommonInterfaceForOneDao(myDao);
    }

    @Test
    public void can_save_item() throws Exception {
        MyItem myItem = new MyItemImpl(UUID.randomUUID(),"Test Item");
        myCommonInterfaceForOneDao.save(myItem);
        assertTrue(myCommonInterfaceForOneDao.exists(myItem.getGuid()));
    }

    @Test
    public void can_delete_item() throws Exception {
        MyItem myItem = new MyItemImpl(UUID.randomUUID(),"Test Item");
        myCommonInterfaceForOneDao.save(myItem);
        myCommonInterfaceForOneDao.delete(myItem.getGuid());
        assertFalse(myCommonInterfaceForOneDao.exists(myItem.getGuid()));
    }
}

In the setup method, you can see that MyInMemoryDao is initialized and then injected into the MyCommonInterfaceForOneDao constructor which then allows these simple unit tests to be run verifying the correct operations. And with this, we’ve wrapped up the second section of the code, creating a data access layer that abstracts the database away from the production code and will allow multiple implementations of the data store to be used with minimal issues.

In later articles, I’ll show how to use Spring to simplify these tests and the application itself. Next time, we’ll talk about the business logic and REST layers of the code.

If you have any comments, concerns, or updates I should make to this, let me know in the comments below.

Running command-line programs from inside Java

I’ve been writing a program for a class I’m taking at school to automate some of the work that had to be done.  I could have written it as a shell script, but I wanted to be more adventurous and write it in Java so it could be run in more than just a Unix environment.  So, as you can guess, I had to do execute the program from the command line.  Normally, you’d do this with the exec method off of the Runtime, however, this causes some problems with permissions.  The way to do execute something from the command line is to use the ProcessBuilder object and it’s dead simple.

List command = new ArrayList();
command.add("file_to_execute");
command.add("option1");
command.add("option2");

ProcessBuilder processBuilder = new ProcessBuilder(command);
Process process = processBuilder.start();

process.waitFor();

As you can tell looking at this simple example, command is a list of String which becomes the command you’d normally type into the command line interface. The processBuilder object sets all the wonderful things up to run an operating system process and when you specify start() it turns it into a Process where you can then wait for all the return value.

Of course, there is a lot more to it than just this, but for my case, all I needed was a quick and dirty command to be run and this did the trick.  You can read more about it on StackOverflow.