Class naming convention and information theory

How many times have you seen classes named with one of these suffixes: manager, util, helper, common, impl etc.? How many times have you seen columns in same DB table called *_id? Probably far too many. 

Have you ever thought about informativeness of above suffixes? Isn't it loosing its point of passing a piece of information to other developers, when it is overused? To be frank, it looks like a noise, a message which is blurred by useless words. The more often those funny words occur in code base, the less information they carry. That is actually, what Shannon said when he was building keystones for information theory.

In fact, it is very important, to give a proper name for a class, DB column name etc. On the other hand, it is also very significant not to overuse same words all over again in names. It is not only getting less readable, but also adding a kind of boilerplate, giving no informativeness value, at all. There is of course a value in pissing other developers off, but I guess it is rather doubtful pleasure ;)

Wrestling with Singleton (Logger) - round 3

Singletons have many faces. One of them is logging mechanism. Before we start having a deeper look into entire case, let's go straight away to a simple example, to set some context.

package com.korczak.oskar.refactoring.singleton.logger.before;

import com.korczak.oskar.refactoring.singleton.logger.Engine;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import static org.mockito.Mockito.verify;
import static org.mockito.MockitoAnnotations.initMocks;

@RunWith(MockitoJUnitRunner.class)
public class CarTest {

 @Mock private Engine engine;
 @InjectMocks private Car car;

 @BeforeMethod
 public void setUp() {
  initMocks(this);
 }

 @Test
 public void shouldRunEngine() {
  car.start();

  verify(engine).run();
 }

 @Test
 public void shouldStopEngine() {
  car.stop();

  verify(engine).stop();
 }
}

package com.korczak.oskar.refactoring.singleton.logger.before;

import com.korczak.oskar.refactoring.singleton.logger.Engine;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class Car {

 private Logger carLogger = LoggerFactory.getLogger(Car.class);

 private Engine engine;

 public Car(Engine engine){
  this.engine = engine;
 }

 public void start(){
  engine.run();
  carLogger.info("Engine is running.");
 }

 public void stop(){
  engine.stop();
  carLogger.info("Engine is stopped.");
 }
}

We can also assume that above snippet is actually a part of, so called, fairly decent application, which is logging everything everywhere, so to speak.

What can you tell about such an application?
Is there anything particular, which is stemming out from our imaginary application?
It may look like to somewhat silly task, but unfortunately it has very deep implications, in terms of design, solving production issues, sometimes performance, overall usage and approach. It is quite a few things tangled to logging, as such.

First of all, let's decompose above snippet with attention to some basic facts:

• Static logger is a singleton
• Breach of unit test definition i.e. system under test is writing to file/console
• Lack of readability to some extent
• A bit of noise in business logic

Now, let's discuss above points a bit further and infer some more, useful information.

Logger is a Singleton

Logger is created via static call and a Singleton is returned. However, it is not that bad, as we are only writing to Singleton and never reading from it. It means we are not exploiting a global state introduced in our JVM by Singleton. On the other hand, it is worth bearing in mind that static call means no seams. There are two direct implications of that: 

• untested production code, as there are no seams
• message will be logged anyway during running tests

Unit test definition breach
That is how we came to the second fact related to unit tests.
Michael Feathers defines unit tests as: 

A test is not a unit test if:

It talks to the database

It communicates across the network

It touches the file system

It can't run at the same time as any of your other unit tests

You have to do special things to your environment (such as editing config files) to run it.

I suppose we can agree that above test is an integration one, rather then a unit sort of thing. Feathers's definition sticks to what people understand under unit test term. 

Why don't we finally admit that so called unit tests, which are logging information to a file/console whatever, are in fact a decent integration tests. Full stop.

What is the business related reason?

Going further one can focus on a business reason in that whole tangled situation and answer a question: is there any business related reason to log particular information in particular place?
The most common answer would be that by logging we mean sending a message from application to whoever is browsing logs. Usually, it would be two main groups of recipients: L1/L2 and L3/testes/developers teams. 
L1/L2 guys would like to see only nasty things happening in a given application e.g. errors, alerts and perhaps warnings. On the other hand, L3/testers/developers would like to see a bit more i.e. states generated by application during its processing etc. It is worth reiterating that none of them is a business person.

When we dig a bit deeper and ask what is the underlying principle of logging, we will probably come to below two points:

• log a state of processed unit
• say 'I was here' or 'I have chosen that path in you application logic'

Above things tell us where we are and are often called diagnostic reasons/purposes. They share a common feature that none of them is driven by any business requirement. Let's be honest, business people are not reading logs. They can only read spreadsheets, but you probably know that already, I suppose.

Readability and noise
Unfortunately, logging affects a bit negatively the overall readability of business logic. It adds additional noise and clutters codebase, when it is overused.

How it might be done?
The easiest might be to imagine an event driven system, where an event comes in, it is being processed and some final response is sent. As it was pointed, logging would be used for diagnostic purposes. If yes, I am questioning the logger usage, almost at all. Why don't we change our applications design so that it can monitor, diagnose and track the state of all, incoming events itself? 

I already can hear answers: 'it is too much effort', 'we don't need it' or better one 'our project is different'. ;)

Partially, it might be true and I agree it is pointless, in some trivial cases, to build errors aware applications for your younger sister's diary application, although still it is worth at least considering it in some larger projects. 
Also, don't get me wrong, if there is a clear business drive for having something logged, just do it. The only thing I am asking to remember is fact that logging is a sort of contract between application and whoever is reading logs. If there is a business need for having some log entry, simply test it. 

When you have self aware system in place and an issue occurs, you may ask your application for some useful 'diagnostic information' (including states) on demand via say REST API or whatever. 

Implementing that sort of approach is in fact a win-win situation for all project stakeholders. Not only L3 and developers are able to deal with errors in processing, but also testers, L1, L2 teams are able to do that. Even BAs start analyzing and solving some burning questions themselves. 

In a long run, it is less work for busy developers, everybody can leverage and use diagnostic information. What's even more important, the knowledge and the responsibility is shared across entire team/stakeholders. It only requires two things: 

1. Stop logging everything in the log files
2. Add sort of issues memory or events state memory to your system

The first step here is to start testing logging in your applications. Try to make logger a collaborator of your class, not a static call and use it where it is really necessary. 
Testing logging enforces thinking about every single message, which might be potentially logged. Additional effort spent on writing test for logger, should actually drive aware developer through all above concerns, helping to understand the business reason behind particular piece of code.

package com.korczak.oskar.refactoring.singleton.logger.after;

import com.korczak.oskar.refactoring.singleton.logger.Engine;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import org.slf4j.Logger;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import static org.mockito.Mockito.verify;
import static org.mockito.MockitoAnnotations.initMocks;
import static org.testng.AssertJUnit.assertEquals;

@RunWith(MockitoJUnitRunner.class)
public class DecentCarTest {

 @Mock private Engine engine;
 @Mock private Logger decentCarLogger;
 @InjectMocks private DecentCar decentCar;

 @Captor private ArgumentCaptor argument;

 @BeforeMethod
 public void initializeMocks() {
  initMocks(this);
  argument = ArgumentCaptor.forClass(String.class);
 }

 @Test
 public void shouldRunEngineAndLogMessage() {
  decentCar.start();

  verify(engine).run();
  verify(decentCarLogger).info(argument.capture());
  assertEquals("Engine is running.", argument.getValue());
 }

 @Test
 public void shouldStopEngineAndLogMessage() {
  decentCar.stop();

  verify(engine).stop();
  verify(decentCarLogger).info(argument.capture());
  assertEquals("Engine is stopped.", argument.getValue());
 }
}

package com.korczak.oskar.refactoring.singleton.logger.after;

import com.korczak.oskar.refactoring.singleton.logger.Engine;
import org.slf4j.Logger;

public class DecentCar {

 private Engine engine;
 private Logger decentCarLogger;

 public DecentCar(Engine engine, Logger decentCarLogger){
  this.engine = engine;
  this.decentCarLogger = decentCarLogger;
 }

 public void start(){
  engine.run();
  decentCarLogger.info("Engine is running.");
 }

 public void stop(){
  engine.stop();
  decentCarLogger.info("Engine is stopped.");
 }
}

You can find entire example on my GitHub account.

Round three is finished.

Awaitility - testing asynchronous calls in Java

Asynchronicity plays an important role in systems we develop, nowadays. Below are examples, where we are using asynchronous operations, almost on a daily basis, in our systems:

• writing to cache with asynchronous write to database behind the scenes or otherwise
• JMS queues or topics, which are asynchronous itself
• retrieving data from external systems in asynchronous way

Asynchronous operations are often used for caching data in order to decrease the number of expensive calls to: database, webservice etc. There are also mutations of above caching approach. For instance, one may combine sending time consuming or complicated algorithmic operations to JMS queue (asynchronous system). Later on, when results of such an intensive operation/algorithm would be available, it might be simply cached in a local in-memory storage. Next time, when user would try to fetch same data, it will be fast and easy for her to retrieve previously calculated results from cache. Such an approach, gives you an ability to build very fast, scaleable, flexible and user friendly applications/GUIs.
However, there is one caveat here. It is quite difficult to test such an asynchronous system. If we like to do it from scratch, we will need to provide some threads' handlers, timeouts and generally deal with concurrency, which makes the whole test more obscure and less focused on business principles ruling the entire system. If above situation is true, it means that we found a niche and basically a need for some library to handle our problem. 
The answer for our headache is Awaitility. It's a quite simple and powerful, Java based library, to test asynchronous calls. Moreover, it does have a concise and expressive DSL to define expectations.

Now, let's see Awaitility in action. I wrote a simple application, which is available on GitHub. The basic idea is that there is a DelayedFileCreator class, which is responsible for creating a file on a filesystem. It also tries to mimic an expensive and time consuming calculations, by time delay. Important thing is a result of that operation. This sort of asynchronous calls are having either a state returned or they cause a change of state somewhere else - in our case it is a newly created file on a filesystem. 

package asynchronousexample;

import java.io.File;
import java.io.IOException;

public class DelayedFileCreator implements Runnable {

 private static final int THREE_SECONDS = 3000;

 private File file;
 private Timer timer;

 public DelayedFileCreator(Timer aTimer, File aFile) {
  timer = aTimer;
  file = aFile;
 }

 @Override
 public void run() {
  sleepBeforeCreatingFile();
  createNewFile();
 }

 private void sleepBeforeCreatingFile() {
  try {
   timer.sleep(THREE_SECONDS);
  } catch (InterruptedException ie) {
   throw new RuntimeException(ie);
  }
 }

 private void createNewFile() {
  try {
   file.createNewFile();
  } catch (IOException ioe) {
   throw new RuntimeException(ioe);
  }
 }
}

 
Ideally, we would like to be able to write a test, which invokes overridden method run(), waits until it is done and asserts the result of asynchronous operation. Awaitility hits the nail on the head. Below example shows an integration test, using nice and readable DSL, to assert the result.

package integration;

import asynchronousexample.AsynchronousTaskLauncher;
import asynchronousexample.DelayedFileCreator;
import asynchronousexample.Timer;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;

import java.io.File;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

import static com.jayway.awaitility.Awaitility.with;
import static com.jayway.awaitility.Duration.ONE_HUNDRED_MILLISECONDS;
import static com.jayway.awaitility.Duration.TEN_SECONDS;
import static com.jayway.awaitility.Duration.TWO_HUNDRED_MILLISECONDS;
import static org.hamcrest.Matchers.equalTo;

public class CreateFileAsynchronouslyIntegrationTest {

 private static final int THREAD_POOL_SIZE = 3;
 private static final String FILENAME = "sample.txt";

 @BeforeTest
 public void deleteFileFromFileSystem() {
  File file = new File(FILENAME);
  if (file.exists()) {
   file.delete();
  }
 }

 @Test
 public void shouldAsynchronouslyWriteFileOnDisk() throws Exception {
  AsynchronousTaskLauncher launcher = prepareAsynchronousTaskLauncher();
  Runnable delayedFileCreatorTask = prepareDelayedFileCreatorWith(FILENAME);

  launcher.launch(delayedFileCreatorTask);

  with().pollDelay(ONE_HUNDRED_MILLISECONDS)
    .and().with().pollInterval(TWO_HUNDRED_MILLISECONDS)
    .and().with().timeout(TEN_SECONDS)
    .await("file creation")
    .until(fileIsCreatedOnDisk(FILENAME), equalTo(true));
 }

 private Runnable prepareDelayedFileCreatorWith(String filename) {
  Timer timer = new Timer();
  File file = new File(filename);
  return new DelayedFileCreator(timer, file);
 }

 private AsynchronousTaskLauncher prepareAsynchronousTaskLauncher() {
  ExecutorService executorService = Executors.newFixedThreadPool(THREAD_POOL_SIZE);
  return new AsynchronousTaskLauncher(executorService);
 }

 private Callable fileIsCreatedOnDisk(final String filename) {
  return new Callable() {
   public Boolean call() throws Exception {
    File file = new File(filename);
    return file.exists();
   }
  };
 }
}

 
The whole beauty lies in readability and expressiveness of Awaitility. We do not have to take care about threads' handling, concurrency aspects etc. Everything is being done by Awaitility. 

I really encourage you to use this small, but very handy library to test your asynchronous calls. 

Which game are you playing ?

Few days back, I spoke with my friend about pretty "funny" situation. He told me that developers at his project are playing a game, which my friend called Pair Programming vs. Code Review. If developers like each other, they play Pair Programming. If they do not like each other, then they play Code Review.

As you can see those developers found a problem, where it was not even existing. That's a rare and undesired skill, which often manifest itself in pseudo pragmatic teams, who are probably not sticking to 5th Agile manifesto principle: 
"Build projects around motivated individuals. 
Give them the environment and support they need, and trust them to get the job done."
Generally, it is called over-engineering! But not only that, they are loosing time for guerrilla warfare, rather than focusing on helping each other. They also seem not to have any respect to each other. 

The question is: should it be this way? Of course, no!
I would like to be clear about Pair Programming and Code Review, so let's start from some definitions.

Scratching Tiger, Hidden Dragon - Pair Programming

One developer types in code at the keyboard. He works at the tactical level. The other one looks for mistakes and thinks strategically about whether the code is being written right and whether the right code is being written. 

You can also describe pair programming in a more holistic and tao oriented way, as a yin-yang or a unity and harmony arousing from diversity. There are many approaches explaining principles of pair programming like coder and observer, driver and navigator, however I am always keen on scratching tiger and hidden dragon parallel. 

In other words, it is a conjunction of opposite features like active and passive, linear and non-linear or specific and general in a collective action such as solving a problem - programming in this case.

To be fairly honest, one can extend and generalize this approach to any sort of task you have got to do. 

The overall idea is very easy to grasp and simple. Let's look at pair programming from two, different perspectives: tiger and dragon.

Scratching Tiger:

1. Active person writing the code, almost scratching it
2. Keeps on focusing on syntax and details
3. Uses his linear part of brain, logical (left) hemisphere to solve arousing problems

Hidden Dragon:

1. Passive person pays attention to code quality and acceptance criteria fulfillment, almost like dragon, flying in the sky and observing what tiger does
2. Can see whole solution, has an overall idea of subsequent tasks and steps
3. Uses his non-linear part of brain, abstract (right) hemisphere to imagine a flow and grasp the bigger picture

You would probably agree that above description is somewhat static and simply a bit boring. Frankly, you are right, but the key here is movement and synergy you can get from it.

How can we get the movement ?

Again, this is simple. You need a proper space. Comfortable desk, two chars and two keyboards. These things are really important, so do not neglect any of it, especially two keyboards. These prerequisites enable higher level of collaboration between developers. Apart from that they are not loosing time and ideas, while swapping keyboards.

And now comes the most important part. Developers are creating a collective mind, which is buzzing and boiling due to new, fresh ideas. The worst thing you can do, is not switching between Tiger and Dragon role. You have to do it frequently, without any hesitation and freely. You cannot be afraid of your partner saying leave that keyboard, etc. When you see Tiger did a typo, initialized a loop in a wrong way or did not do refactoring, which might be done - simply take over and use your own keyboard to fix it. You need to change perspectives very often and switch your brain from linear to non-linear mode and vice versa. That is the only way you can solve difficult problems. 

Why pairing on solving tasks?

Mainly because of four reasons:

• Readability - two people take care about readable code, both of them have to understand what was written

• Code quality - percentage of defects found 40% - 60% (Steve McConnell "Code Complete")

• Bus factor - knowledge sharing and mitigating the risk of single point of failure

• Pressure - group shields developers from external pressure much better than single developer can protect himself. What's more, in stressful situations people tend to cluster to face difficult situation.

Okay, so when it is worth to pair? 

It is definitely more expedient to pair on crucial and strategically important tasks. It is also useful to pair on both very hard, non-trivial implementations and medium level difficulty tasks. There is no use of pairing on relatively easy tasks, as they do not add so much value, comparing to effort.

Always remember about rotating pairs, not giving an opportunity to get used to your partner. The efficiency is decreasing then. Moreover, it is also good not to pair newbies, as there is not master-apprentice relation, which negatively affects pairing process. Definitely people need to like each other.

Code Review - No junk in a trunk

It is a sort of semi formal walkthrough the code aiming to:

1. share knowledge (mitigate bus factor)
2. check design from high level perspective
3. check at least SOLID rules appliance
4. check definition of done (DoD)
5. increase resilience before holidays

When can you perform code review?

Of course after development and testing, but before merging to master or trunk, so that there is no junk in a trunk.

The programmer or pair, who developed a solution, walks some other team member through new code. They start from telling the story and acceptance criteria, then they are explaining applied design and showing working, green, CI  system tests. There is also a place for Q&A and potentially some code review remarks or changes. Code review is able to discover between 20% to 40% of defects (Steve McConnell "Code Complete").

Summary

Code Review and Pair Programming are two, complementary techniques improving the general quality of the provided solution. Apart from that they mitigate the risk associated with bus factor, by knowledge sharing.

Both of them create a useful unity in development cycle. 

You may even use scratching tiger, hidden dragon parallel, in this case, as well. Pair Programming would be a tiger and code review a dragon looking at the solution from the distance. 

It is much cheaper (sic!) to let developers be tigers and dragons, rather than giving them a wigging that they loose time for crap.

Rarely you have to choose between these two approaches. Then do it wisely and choose pairing. If your management is trying to be clumsy lean oriented and tells you should not do pairing, as it is a pure waste, take at least code review. Remember, half pairing is still better than not paring at all. It is always good to discuss design, start doing the most tricky bit together and then split and finish what you started separately, if time is so valuable. 

However, in fact it does not hold true. You are writing robust and clean solutions faster while paring and reviewing, than not. 

Golden rule of readability

Golden Rule:
Readability on a first place. It is a general rule in software engineering. Of course, everything should be done with a dose of common sense.

Paraphrased by extremists:
It is better to have a readable and not working code, rather than non readable and working one. 

Explanation:
One is able to fix readable code easily. Although the second statement says the code works, it is still not readable!