Androidsx is now Perops. Visit us at perops.com

Androidsx Androidsx | android and wearable developers

 

Subscribe for market insights and new posts

 

Stop the spread of nulls

October 4, 2010 at 7:38 pm | blog, software quality | 3 comments

 

Say we want to write an Android application that listens to everything you say, and tries to find the information that you might need before you even ask for it. For instance, you ask your friend over the phone Is there any good pizzeria near your place?, and this application will perform a query against Google Maps using your friend’s address retrieved from your phone contacts and provide you with a couple of recommendations.

To parse the user commands, after converting the speech to text, and provide the appropriate action handlers, we have these high level interfaces:

public interface Parser {
    Action findAction(String userInput);
}

public interface Action {
    void performAction();
}

There are two proposals for the implementation of the parser:

// Altenative A
class DbBasedParserA implements Parser {
    @Override
    public Action findAction(String userInput) {
        // Find the appropriate action in the DB
        return actionFound ? theAction : null;
    }
}

// Altenative B
class DbBasedParserB implements Parser {
    private static final Action DO_NOTHING = new Action() {
        @Override
        public void performAction() {
            // Do nothing
        }
    };

    @Override
    public Action findAction(String userInput) {
        // Find the appropriate action in the DB
        return actionFound ? theAction : DO_NOTHING;
    }
}

What are the consequences of this choice? Let’s see the code of the clients [1]:

// Client for alternative A
Parser parser = ParserFactory.getParser();
if (parser == null) {
    throw new IllegalStateException();
}

Action action = parser.findAction(someInput);
if (action == null) {
    // Do nothing
} else {
    action.performAction();
}
// Client for alternative B
ParserFactory
    .getParser()
    .findAction(someInput)
    .performAction();

And this is contagious. Those if (action == null) are going to keep on spreading over the different layers of the system unless some API stops it. Help save lines of code and NPEs by ensuring no nulls are passed around!

[1] There is also a ParserFactory, with two implementation alternatives that react differently when no parser is found: one returns null, while the other throws unchecked exceptions. Back to post.

 

How to write untestable code

January 14, 2010 at 11:08 am | blog, software quality | 2 comments

 

Say that we write code for a machine that it so powerful that rumors say it could generate a black hole. We need to be serious about security: we cannot let those evil international terrorists find out our black-hole-making secrets and blow half the Milky Way Galaxy.

If they managed to access our source code, they could extend it and adapt it to their cruel use case, which would represent a huge threat for the humankind. In order to reduce risks, we should write code that is difficult to understand, to maintain and of course, to test. Here’s how:

  • Depend on concrete classes – Tie things down to concrete classes – avoid interfaces wherever possible: they let people substitute the concrete classes you’re using for their own classes which would implement the same contract in the interface. By depending on concrete implementation, we make sure that they’ll have a hard time testing whether their evil plans will release enough energy to toast their breakfast bread.
  • Make your own dependencies – Instantiate objects using new in the middle of methods, don’t pass the object in. Anyone that wants to test that code is forced to use that concrete object you new’ed up: they can’t inject a dummy, fake, or other mock in to simplify the behavior or make assertions about what you do to it.
  • Conditional slalom – Feel good when writing lengthy if branches and switch statements. These increase the number of possible execution paths that tests will need to cover when exercising the code under test. The higher the cyclomatic complexity, the harder it is to test and understand! Why use polymorphism instead of conditionals? Don’t make it so easy! Make the branching both deep and wide: if you’re not consistently going at least 5 conditionals deep, the patient terrorists might even find out what the code intends to do under all possible combinations.

    It is also known as the Arrow Antipattern:

    if a
        if b
            if c
                foo();
            else
                bar();
            endif
        endif
    endif
    

    Refactoring into something more suitable is usually possible:

    if a && b
        if c
            foo();
        else
            bar();
        endif
    endif
    
  • Use global flags – Why call a method explicitly? Set a flag in one part of your code, in order to cause an effect in a totally different part of your application. The testers will go crazy trying to figure out why all of a sudden a conditional that was evaluating true one minute is all of a sudden evaluating to false. Not to mention the mess that using that system becomes: if we manage to keep the wikis safe from their hands, they’ll have a very hard time figuring out what system properties to set in order to get the deadly weapon working.
  • Loop-switch sequence – By means of which a clear set of steps is implemented as a byzantine switch-within-a-loop. Fulfilling our mission to obfuscate the code , is much more difficult to decipher the intent and actual function of the code than the more straightforward refactored solution.

    This is a terrorist-safe code snippet:

    // parse a key, a value, then three parameters
    String key = null;
    String value = null;
    final List params = new LinkedList();
    
    for (int i = 0; i < 5; i++) {
      switch (i) {
        case 0: key = stream.parse(); break;
        case 1: value = stream.parse(); break;
        default: params.add(stream.parse()); break;
      }
    }
    

    While this is too easy to understand:

    // parse a key and value
    final String key = stream.parse();
    final String value = stream.parse();
    
    // parse 3 parameters
    final List params = new LinkedList();
    for (int i = 0; i < 3; i++) {
      params.add(stream.parse());
    }
    

References: This post on the Google Testing Blog, the Arrow AntiPattern in Cunningham & Cunningham webpage.

 

Don’t look for things. Ask for them!

November 26, 2009 at 12:04 pm | blog, software quality | 5 comments

 

Before our previous article, our code to model a mechanic used to look like this:

class Mechanic {
    private final Engine engine = ServiceLocator.getCar().getEngine();
    public void fixEngine() { /* ... */ }
}

Luckily, we applied some dependency injection, resulting in nicer code: testable, and with an explicit dependency on the ServiceLocator:

class Mechanic {
    private final Engine engine;
    public Mechanic(final ServiceLocator serviceLocator) {
        engine = serviceLocator.getCar().getEngine();
    }
    public void fixEngine() { /* ... */ }
}

However, does our mechanic care about the ServiceLocator at all? Not really, it doesn’t even store a reference to it! The mechanic just wants an engine.

There are some problems with this kind of code:

  • The ServiceLocator probably has references to a lot of other classes in the system. By the transitive property, our Mechanic class is too coupled with the rest of the system: if you want to reuse it in a different project, you’d need to reference not only Mechanic and Engine, but also the ServiceLocator with all its dependencies.
  • The API is not clear. An API client knows that it’d need to provide the ServiceLocator (already an advantage respect to the Singleton-based case). What it doesn’t know is what the mechanic really needs (an engine). This fact is hidden in the source code.
  • The tests contain a lot of setup junk that masks its real purpose. For every test of the mechanic, you’d need to mock the ServiceLocator, and then make sure the appropriate reference can be retrieved from it (the Engine, in this case). When we test the class Garage, that needs a Mechanic, we’ll have to replicate this tedious setup there too.

This is how the test looks like now:

@Test
public void testCantFixBrokenDownEngine() {
    final Engine engine = EngineFactory.buildBrokenEngine();
    final Car mockCark = Mockito.mock(Car.class);
    Mockito.when(mockCar.getEngine()).thenReturn(engine);
    final ServiceLocator mockServiceLocator = Mockito.mock(ServiceLocator.class);
    Mockito.when(mockServiceLocator.getCar()).thenReturn(car);
    final Mechanic mechanic = new Mechanic(mockServiceLocator);
    mechanic.fixEngine();
    assertFalse(engine.works());
}

Why not just ask for what you need?

class Mechanic {
    private final Engine engine;
    public Mechanic(final Engine engine) {
        this.engine = engine;
    }
    public void fixEngine() { /* ... */ }
}
@Test
public void testCantFixBrokenDownEngine() {
  Engine engine = EngineFactory.buildBrokenEngine();
  Mechanic mechanic = new Mechanic(engine);
  mechanic.fixEngine();
  assertFalse(engine.works());
}

Everyone wins:

API writers:

  • Unaffected by ServiceLocator changes
  • In-code documentation is easier to write (what was your comment on the Mechanic‘s constructor for the ServiceLocator)?

Test writers:

  • Tests are easy to read
  • Very little setup code
  • Unaffected by ServiceLocator changes

API users:

  • The API is clear: the mechanic needs an engine
 

Can’t test that Singleton? Try Dependency Injection!

November 11, 2009 at 9:40 pm | blog, software quality | 4 comments

 

So you want to test this method:

public class Client {

    public int process(Params params) {
        final Server server = Server.getInstance();
        final Data data = server.retrieveDate(params);
        // do stuff
    }
}

We don’t want to retrieve an instance of a real server for our little unit-test, so how can we test this method?

It is hard to test code that uses singletons.

We don’t control the creation of the singleton object, as it is performed inside a static method. There is no way to mock the object in order to test the behavior of our method in isolation.

Refactor it to use Dependency Injection.

You can refactor Client to avoid using the singleton pattern. Instead of obtaining the Server instance from the static getInstance() method, allow Client to accept it through its constructor.

public class Client {
    private final Server server;  

    public Client(Server server) {
        this.server = server;
    }  

    public int process(Params params) {
        final Data data = server.retrieveData(params);
        // do stuff
    }
}

Let’s write that test now:

@Test
public void testConnectionUpTime() {
    final Server mockServer = Mockito.mock(Server.class);
    final Params params = // ...
    Mockito.when(mockServer.process(params)).thenReturn(5);
    final Client client = new Client(mockServer);
    assertEquals(5, client.process(params));
}

The code is now both clearer and testable.

The dependency between the client and the server is now explicit: Client client = new Client(server);. There is no way a developer creates a client instance without noticing that a server instance must be configured: it is a parameter in the constructor.

The singleton allowed to create a client instance without configuring the server in advance. The object would be successfully created and the application would execute, until one of the methods runs into a non-configured/non-reachable/null server and fail at runtime :’-(

 

Categories:

Recent posts:

Search:

Subscribe