Nobody expects the unexpected invocation

  
unexpected invocation: someObject.someMethod()
no expectations sepecified: did you...
- forget to start an expectation with a cardinality clause?
- call a mocked method to specify the parameter of an expectation?
what happened before this: nothing!

This one always gets me. I don't know why as I've worked with jMock long enough now to know how to type it out, but this is invariably the problem:

  
context.checking(new Expectations() {{
 oneOf(mockObject.mockMethod()); will(returnValue(expValue));
}});

Maybe it's years using EasyMock, but I nearly always place my brackets wrong and forget to close the cardinality clause, so the above becomes:

  
context.checking(new Expectations() {{
 oneOf(mockObject).mockMethod(); will(returnValue(expValue));
}});

Remember - oneOf(object), not oneOf(object.method). D'oh.

Listen to the Words I'm Saying With My Mouth

Multithreaded code is at the crux of any server developers toolbox and alot of times the producer/consumer pattern is used. I've seen a lot of implementations of this and typically the developer will use a shared queue for communication.

To draw an example, if we were to write a process that will read lines from a file, reverse each line and write it to another file, we might have something like this:-

  
public class QueueBasedReverser {
 private static final int QUEUE_SIZE = 100;

 public static void main(String[] args) throws IOException {
  FileReader reader = new FileReader("inputfile");
  FileWriter writer = new FileWriter("outputfile");
  
  ArrayBlockingQueue<String> queue = new ArrayBlockingQueue<String>(QUEUE_SIZE);
  
  LineProducer producer = new LineProducer(queue, reader);
  LineConsumer consumer = new LineConsumer(queue, writer);

  new Thread(producer).start();
  new Thread(consumer).start();
 }
}
 
public class LineProducer implements Runnable {
 public static final String POISON_PILL = "_THIS_IS_THE_POISON_PILL_MESSAGE";
 private final BlockingQueue<String> queue;
 private final BufferedReader reader;

 public LineProducer(BlockingQueue<String> queue, FileReader reader) {
  this.queue = queue;
  this.reader = new BufferedReader(reader);
 }

 @Override
 public void run() {
  String line;
  try {
   while ((line = reader.readLine()) != null) {
    queue.put(line);
   }
   queue.put(POISON_PILL);
   reader.close();
  } catch (Exception e) {
   throw new RuntimeException(e);
  }
 }
}
 
public class LineConsumer implements Runnable {
 private final BlockingQueue<String> queue;
 private final BufferedWriter writer;

 public LineConsumer(BlockingQueue<String> queue, Writer writer) {
  this.queue = queue;
  this.writer = new BufferedWriter(writer);
 }

 @Override
 public void run() {
  String line = null;
  try {
   while ((line = queue.poll(1, TimeUnit.MINUTES)) != LineProducer.POISON_PILL) {
    writer.write(reversed(line));
    writer.newLine();
   }
   writer.close();
  } catch (Exception e) {
   throw new RuntimeException(e);
  }
 }

 private String reversed(String line) {
  return new StringBuffer(line).reverse().toString();
 }
}

This is a fairly typical implementation making use of a shared bounded queue for communication and a poison pill to signify the end of processing. This is problematic as the queue and the poison pill are both leaking implementation details about both the producer and consumer and has tied them both together quite tightly.

Those who have some experience with UI programming should be well versed in the notion of adding listeners to be notified of when events happen, and this is a pattern we can apply on the server as well as the client. The idea here is that the producer raises an "event" when it wants to tell the world something, in this case that it has read a line from the file. The consumer can register itself as a listener for this event and respond by writing the reversed string to the output file.

 
public class ListenerBasedReverser {
 public static void main(String[] args) throws IOException {
  FileReader reader = new FileReader("inputfile");
  FileWriter writer = new FileWriter("outputfile");
  
  LineProducer producer = new LineProducer(reader);
  LineConsumer consumer = new LineConsumer(writer);
  producer.addListener(consumer);
  producer.readFile();
 }
}

public interface LineReadListener {
 void lineRead(String line);
 void allLinesRead();
}

public class LineProducer {
 private final BufferedReader reader;
 private ArrayList<LineReadListener> listeners = new ArrayList<LineReadListener>();
 
 public LineProducer(FileReader reader) {
  this.reader = new BufferedReader(reader);
 }

 public void readFile() {
  String line;
  try {
   while ((line = reader.readLine()) != null) {
    for (LineReadListener listener : listeners) {
     listener.lineRead(line);
    }
   }
   reader.close();
   for (LineReadListener listener : listeners) {
    listener.allLinesRead();
   }
  } catch (Exception e) {
   throw new RuntimeException(e);
  }
 }

 public void addListener(LineReadListener consumer) {
  listeners.add(consumer);
 }
}

public class LineConsumer implements Runnable, LineReadListener {
 private static final String POISON_PILL = "_THIS_IS_THE_INTERNAL_POISON_PILL_";
 private final BlockingQueue<String> queue;
 private final BufferedWriter writer;

 public LineConsumer(Writer writer) {
  this.queue = new ArrayBlockingQueue<String>(100);
  this.writer = new BufferedWriter(writer);
  new Thread(this).start();
 }

 @Override
 public void run() {
  String line = null;
  try {
   while ((line = queue.poll(1, TimeUnit.MINUTES)) != POISON_PILL) {
    writer.write(reversed(line));
    writer.newLine();
   }
   writer.close();
  } catch (Exception e) {
   throw new RuntimeException(e);
  }
 }

 private String reversed(String line) {
  return new StringBuffer(line).reverse().toString();
 }

 @Override
 public void lineRead(String line) {
  try {
   queue.put(line);
  } catch (Exception e) {
   throw new RuntimeException(e);
  }
 }

 @Override
 public void allLinesRead() {
  try {
   queue.put(POISON_PILL);
  } catch (Exception e) {
   throw new RuntimeException(e);
  }
 }
}

In this implementation we're still using a bounded queue and a poison pill, but this is now completely internal to the consumer and we have the option to change the implementation without touching any other part of the system. The producer and consumer have no shared information or implementation.

The keen eyed among you may have noticed that the actions of the producer and consumer are now also decoupled, meaning the producer has no knowledge that input is being read from a file, it can come from anywhere! The LineProducer also has the ability to be used to read any file, with different listeners attached to do processing.

Another not so obvious benefit of this approach is we can add multiple listeners. If we want to track how many lines per second we were processing, we can just implement a new LineReadListener to handle this logic and add it as a second listener without affecting the line reversing. This gives us a really nice separation of concerns and lets us adhere to the Single Responsibility Principle alot easier.

Follow the Brick Road

Recently I was writing a training course on unit testing and wanted to drag some real-world examples in from our existing codebase. A fairly simple task you would think; just copy the files you need and any immediate dependencies and off you go.

And therein lies the crux of the problem. Dependencies.

As I was teaching about unit testing all I really needed was the class under test and any data beans it would need to get it's job done. 2 or 3 classes tops. So I start by bringing in the first dependency, then the dependencies that class has, then the dependencies those classes have. After I got to 320 classes I gave up and went with different approach.

Remember the big ball of mud I showed you previously, well there you have a living, breathing example of it.

The class I was testing was a wrapper over a 3rd party library so itself should be a fairly straightforward piece of code, so why on Earth was it reaching back into the core code so grotesquely? Because the classes it made use of were concrete dependencies.

How do we stop this from happening and get back to a nice, loosely coupled system? It can be fairly simple really; just introduce an interface here or there and inject it instead of instantiating a concrete implementation.

public void SavePersonAction {
   public void savePerson(Person person) {
       new PersonDao().save(person);
   }
}

public class PersonDao {
   public void save(Person person) {
       // Db code here
   }
}

Becomes...

public void SavePersonAction {
   private PersonDao dao;

   public SavePersonActon(PersonDao dao) {
       this.dao = dao;
   }
   public void savePerson(Person person) {
       dao.save(person);
   }
}

public interface PersonDao {
  public void save(Person person);
}

public class DbBasedPersonDao implements PersonDao {
   public void save(Person person) {
       // Db code here
   }
}

Easy eh?

When you're sat there coding away and think to yourself “do I need an interface for this?”, if your code is providing a service, behaviour or access to something- in essence, anything that isn't a data object, work to an interface.

What does it cost you? Virtually nothing.
What does it give you? Plenty.

A common thought process I see is to only introduce an interface when there is the possibility of multiple implementations, and sure, you have to do it then but why stop there?

Working to an interface allows you, the developer, to specify exactly what an implementing class should do. You get to specify the contract, rather than relying on the default behaviour provided by the only implementation.

In the case study above a single well placed interface injected into the class would have enabled me to just copy 1 class and 2 interfaces instead of half the codebase. Surely that's got to be a win?

Perhaps, Perhaps, Perhaps

Pop quiz: To you, what's the most important aspect of writing maintainable code?

Using the right language?
Writing unit tests?
Mercilessly refactoring?
Applying design patterns?

All are important, but in my eye the most important aspects are coupling and cohesion. And yes, I know technically that's two things but they're so tightly coupled (haha!) that really you can count them as one.

If you don't know what either of these are, I suggest you look it up on Wikipedia (and pick up a book or two on writing software). Go on, we'll wait for you. And we promise not to snigger as you leave.

...

Achieving a state of loose coupling and high cohesion is simple really:

1. If you're writing a method, ensure that it's contributing directly to the goal of the class and think very carefully about what your class is supposed to be doing


2. Ensure your class only ever does one thing


3. Work to interfaces, not implementations

Ok, ok, so it's not easy otherwise we wouldn't need to be having this conversation, but that's an article for another day.

Alot of code I've seen, and even been guilty of writing in the past doesn't adhere to these principles and over time turns into a Big Ball of Mud, which if you take a conceptual view of how the system fits together might look something like this...





That's right, just a big box with you having no idea about what's going on. Every piece of code knows about everything other piece of code and it may as well be a behemoth piece of procedural code all stored in the main method accessing global data.

I'm probably being unfair and putting forward an extreme example. This is alot closer to reality....





A system with a massive amount of dependency, and very representative of what you'll typically see in alot of codebases.

What we really want to see is a collection of small, independent pieces of code all working together to replace the “Master Control Program” above...




Hmm, much nicer. And believe it or not, it's achievable.

"But why is this important", I hear you cry?

One word: Maintenance.

The thing that every developer hates, but every developer has to do*. Writing code is easy. Writing code that you can come back to 6 months later and instantly figure out what was going on in your head is a lot more difficult. What does that 1600 line method actually do? How does it do it? Why does it invoke the Foobatron and send the results through a tea strainer? Highly cohesive, loosely coupled code helps you with this. Even if you can figure it out, how about the next poor schmo who has to come and work with your code? How about when you need to start getting some data from an FTP site instead of directly from disk. It's going to be alot easier when all you need to do is provide a new implementation of a FileRetriever and exchange the dependency than it is to delve in at line 459 of the “readFileParseDataAndUpload” method.

Given a choice between debugging a one million line method or a five line method, which would you choose?

* if you never have to do this, please drop me a line about where I can send you my cv!

I Do It Because I Have To Keep Up

Have you ever seen that episodes of scrubs where Dick van Dyke plays Dr Townshend? No? Well it tells the story of an ageing doctor who's using out of date medical practices and gets called on it. You know what, I'll let the story be told as it was intended...

Dr. Kelso: I'm sorry, buddy. Say, listen, uh...nowadays it has become kind of hospital protocol to do modified Seldinger in a case like this. You _do_ know how to do one of those, don't you?

Dr. Townshend: 'Course!

Dr. Kelso: Good. Because the patient in bed number 2 needs one. You mind doing it for me?

Dr. Townshend: What the hell's this all about?

Dr. Kelso: Nothing! I was, uh, just looking over your files and, um...well...your osteoporotic patients aren't on Bisphosphonate; your diabetics aren't on ACE inhibitors. Doug, a lot of your treatments are pretty out of date.

Dr. Townshend: Come on, Bob, I'm--guys like us, we're set in our ways.

Dr. Kelso: Well, this is not an age thing, Doug. Hell, these days if you've been out of med school five years, half of what you learned is obsolete. Why do you think I spend every other weekend at a seminar in some two-star hotel ballroom that still stinks of last night's prom vomit? I do it because I have to keep up.

Dr. Townshend: Also, it gives you two days away from the missus, right? [laughs] Once again, I am sorry I was the one who introduced you in the first place!

Dr. Kelso's expressions remains serious.

Dr. Townshend: Look, Bob, I just...I don't have the energy for all that stuff.

Dr. Kelso: Well...then we got a problem

What's this got to do with software development? Let's pretend the above exchange was 2 developers chatting....

Alice: I'm sorry, buddy. Say, listen, uh...nowadays it has become kind of standard practise to use dependency injection in a case like this. You _do_ know how to do that, don't you?

Bob: 'Course!

Alice: Good. Because the code in the accounting module needs it. You mind doing it for me?

Bob: What the hell's this all about?

Alice: Nothing! I was, uh, just looking over your source files and, um...well...your database code is open to SQL injection attacks; your classes are large and in-cohesive. Bob, a lot of your techniques are pretty out of date.

Bob: Come on, Alice, I'm--developers like us, we're set in our ways.

Alice: Well, this is not an age thing, Bob. Hell, these days if you've been out of uni five years, half of what you learned is obsolete. Why do you think I spend every other weekend at a seminar in some two-star hotel ballroom that still stinks of last night's prom vomit? I do it because I have to keep up.

Bob: Also, it gives you two days away from the husband, right? [laughs] Once again, I am sorry I was the one who introduced you in the first place!

Alice's expressions remains serious.

Bob: Look, Alice, I just...I don't have the energy for all that stuff.

Alice: Well...then we got a problem

Doctor's have to constantly keep up with the latest techniques and practices as do accountants, lawyers, engineers, and countless other professionals. Software permeates every facet of modern life, so why do we get away with it?

Ask yourself this, if you had to pass an exam set by a professional body, would you be able to do it without reading alot of books first? Off the top of your head Can you explain what the Single Responsibility Principle is and why it's important? Liskov Substitution Principle? The Law of Demeter? Dependency Injection? How about why Singletons are evil? Can you give me 5 benefits of unit testing or what effects test-driven development have on software design?

Perhaps you're fresh out of university with a CS degree under your belt and you think you know everything. Maybe you've been in the industry 20 years and realise you know nothing; have you ever asked yourself how much you really know about programming?

Virtually nothing that I'm going to tell you is original, but then again unless you're at the bleeding edge, not a lot is these days. What I'm going to do is distil some of the information and techniques I've learned in 10 years of being a professional software developer. You might disagree with some, or a lot, of what I have to say and that's great, I'd love to hear what you have to say, perhaps we can learn something from each other.