Duplication is evil, double duplication is worse

Think of an application with a domain model in which a person can have several emails, each of which has a different role (e.g. private, office, preferred, and so on). Persons can choose their preferred email for normal communications, but there are other communications that are normally sent to the email with a given role (e.g. communications about personal health will not be sent to an email which is also read by secretaries); only when an email with this role is missing the system reverts to a default email, which is chosen between the existing ones applying a chain of rules.

In an application like this you could (hypotetically speaking, of course) find a snippet of code that returns the email corresponding to a given role:

public String getEmailForRole(Role role) {
 String email = emails.get(role);
 return email != null && isValidEmail(email)
     ? email
     : getDefaultEmail();
}

public boolean isValidEmail(String pattern) {
 return (...stuff...);
}

The isValidEmail method checks the email against a simple regular expression. This snippet springs at least two different considerations.

Check for correctness

One might think that the check for validity is unnecessary, after all checks are done while inserting and updating, right? Partially. Let's just say all checks should be done there, and let's not forget that data greatly outlive applications, so you could easily have strings with completely different meaning stored where only emails should be. Sounds ugly? Welcome to reality. Sure, you could quite easily find all instances of strings that are not emails, but when the customers asks you to leave stuff as it is you have very little power. Other things I've seen include courses descriptions instead of teachers and documents instead of relatives. To cut a long story short, better safe than sorry.

Be clear on intents

Stick with them and hide mechanics. In particular, here we have a duplicated duplication (pun intended):

• the check for null values must be duplicated in every snippet that contains emails.get(role)
• the check duplicates the intent of the isValidEmail method. I think this duplication is worse than the previous one, first of all because the redundancy blatantly hits the eye, but most of all because there is a conceptual duplication: the responsibility for the check should lie in the idValidEmail method itself.

If there's an isValidEmail(String pattern) method I expect it to check for null values without having to bother myself:

public String getEmailForRole(Role role) {
 String email = emails.get(role);
 return isValidEmail(email)
     ? email
     : getDefaultEmail();
}

public boolean isValidEmail(String pattern) {
 return pattern != null && (...stuff...);
}

A more elegant solution could be the creation of an immutable Email value object and the use of the NullObject pattern, a special case of the more general Special Case pattern (another pun intended); in this case the emails object would return a NullEmail object that implements the same interface of the Email class:

public Email getEmailForRole(Role role) {
 Email email = emails.get(role);
 return email.isValid
     ? email
     : getDefaultEmail();
}

A simplistic implementation of the NullEmail object could be

class NullEmail {

 private Role role;

 public NullEmail(Role role) {this.role = role;}

 public String getValue() {return "";}

 public Role getRole() {return role;}

 public boolean isValid() {return false;}
}

If we wanted to explicitly return a String instead of an Email object we could slightly modify the getEmailForRole method:

public String getEmailForRole(Role role) {
 Email email = emails.get(role);
 return email.isValid
     ? email.getValue()
     : getDefaultEmail();
}

As you might have noticed, nothing changed in our NullEmail class - nor in the Email class, which is not shown here.

Of course in this particular case we still have to check for the validity of the email, but in many other places (e.g. print emails of a person for every possilbe role) we can safely operate on our NullObject just as we would on a valid one.

It could be overkill, but it surely gets rid of one of the worst plagues in software: duplication. But that's the subject for another post :-)

How to use Java to call an IBM i program

This post is a late update to a post I wrote a couple of years ago about exposing RPGLE programs so that we could call them from our webapps. That worked fine, but I didn't particularly like the workflow we used to produce the wrapper, so I tried to get rid of some muda.
The worst one was, in my opinion, the need to use two different IDEs, thus necessarily involving the collaboration of two persons (not every developer has IBM tools) that creates a bottleneck.
After some researches I found some official documentation (the problem with IBM is not scarcity but abundance), dug for our jt400.jar and started to experiment a little.

A small word of caution: the article is written in an introductory style, showing the TDD process that lead to the implementation of the functionality I needed. If you're only interested in the results you can easily skip to the end of the article (I will not know it, so I won't get offended).

The guinea pig for my experiment is a simple program that given a table name returns a serial number which is unique for that table. As I had have to use this program in an application that already uses a serial number generator, first I need an interface, so I use the NetBeans Extract Interface refactoring on my existing service:

public interface SerialsService {
    long getSerialNumber(String tableName);
}

That done I create an implementing class that gets the serial from the iSeries:

public class MyIseriesSerialsService implements SerialsService {
    public long getSerialNumber(String tableName){
        throw new UnsupportedOperationException();
    }
}

OK now we can start the real stuff. First thing first, we write a couple of simple tests in a new test class:

@Test(expected = "IllegalArgumentException.class"
public void testWithWrongTableNameShouldThrowException() {
    MyIseriesSerialsService instance = new MyIseriesSerialsService();
    instance.getSerialNumber("WrongName");
}

CTRL + F6 to start the test and NetBeans gives me a red bar. I change the type of exception thrown, CTRL + F6 again and here's a nice green bar. That was easy, wasn't it? now we have a service that behaves correctly when you don't invoke it correctly. This is good, but if your application is like mine I suppose you expect your service to behave correctly even when it is correctly invoked (yes, sometimes it happens).

To call an IBM i program you first have to connect to an iSeries, and for this you use the AS400 class, which IBM documentation actually calls AS400 IBM® Toolbox per Java™: I'll stick to that, so that is what I mean when I simply write AS400 - the same holds true for all other registered trademarks. If this is enough to keep lawyers at bay, as I hope, this ends the disclaimer.

The AS400 class, amongst other things, manages socket connections on behalf of a user, so at least we have to tell our service what is the iSeries we want to connect to, and which user we want to impersonate. This requires to change the constructor of the service (obviously it is not the only way, but I'd rather use an immutable object). We could pass in the name of the server (or the ip address if you don't want to rely on a DNS service), a username and a password, or we could create an AS400 object and pass it to the constructor of the service. This option separates responsibilities better (and can be tested more easily), so I choose it and write a simple test to check the connection to our system:

@Test
public void testConnection() throws Exception {
    AS400 system = new AS400(host, username, password);
    assertNotNull(system.getRelease());
    System.out.println(system.getRelease());
}

I am just experimenting, so as always I defer all the exception dealings. Green bar. I can now add a field to the constructor of the service...

public class MyIseriesSerialsService implements SerialsService {

    private AS400 system;

    public MyIseriesSerialsService(AS400 system) {
        this.system = system;
    }

    public long getSerialNumber(String tableName) {
        throw new IllegalArgumentException();
    }
}

...and modify the tests accordingly, also removing a small duplication:

@Before
public void setUp() {
    instance = new MyIseriesSerialsService(createValidSystem());
}

private AS400 createValidSystem() {
    return new AS400(host, username, password);
}

@Test(expected = IllegalArgumentException.class)
public void testWithWrongTableNameShouldThrowException() {
    instance.getSerialNumber("WrongName");
}

Skipping (for the sake of the article) all tests on invalid systems and other exceptions we can now start writing a more interesting test:

@Test
public void testGetSerialNumberReturnsPositiveNumber() throws Exception {
    long result = instance.getSerialNumber(tableName);
    assertTrue(result > 0);
}

CTRL + F6, red bar (as expected). This is where the rubber hits the road.

The MYPGM program is contained in the MYLIB library in the QSYS, and it also uses the MYOTHERLIB library (by the way, watch for the maximum length of the names of the libraries, which is 10). The wrapper class for a program call (the Command pattern in action) is ProgramCall (fantasy does not abound in IBM, but that's good as I easily found what I was looking for). It needs to know the system we're operating on, the path to the program and the parameter list. We have the system, so we need the path to the program, for which we use the QSYSObjectPathName class, and the parameter list, for which we use the ProgramParameter class.

The QSYSObjectPathName constructor takes as parameters the name of the library, the name of the program and it extension. Once we have a QSYSObjectPathName object we can ask it the path to the program, which is what precisely what we need:

QSYSObjectPathName pgmName = new QSYSObjectPathName(myLib, myPgm, pgmExtension);

The parameter list is actually an array of ProgramParameter objects, which cannot be null. Each parameter wraps an array of bytes, so we have to use the proper converter; luckily there are some converters ready for us.

Our program has an input/output parameter (the name of the table) and an output parameter (the serial number for the given table), so we have the following:

ProgramParameter[] paramList = new ProgramParameter[2];

The first parameter is the name of the table, which is a String. To deal with it we can use the AS400Text converter:

AS400Text textConverter = new AS400Text(10, system);
byte[] key = textConverter.toBytes(tableName);
paramList[0] = new ProgramParameter(key);

The first parameter for the constructor is the length of the IBM i text, the second one is the system we're operating on.

The output parameter is a long, so we'll have to use a different converter. Up to now let's just initialize it:

paramList[1] = new ProgramParameter(32);

Our command is now ready to be born:

ProgramCall pgm = new ProgramCall(system, pgmName.getPath(), paramList);

To execute it we simply call the run method, which returns a boolean. If the result is true we can extract data from the output parameter and convert it:

byte[] data = paramList[1].getOutputData();
AS400PackedDecimal pdconverter = new AS400PackedDecimal(12, 0);
long result = ((BigDecimal) pdconverter.toObject(data)).longValue();

And this is it. Let's see:

Testcase: testGetSerialNumberReturnsPositiveNumber(testserialias.MyIseriesSerialsServiceTest):
Caused an ERROR
6

Wow that's illuminating... Luckily we aldready know what's going on: if you check above you'll notice I wrote that the program needs another library, which is not loaded when we first connect to the system. To add the library we need to issue a command, thus we use the CommandCall class:

CommandCall cc = new CommandCall(system);
cc.setCommand("ADDLIBLE " + myotherLib);
cc.run();

Now we're going somewhere... As we trust but also want to control we add another simple test to check that the service returns bigger numbers on consecutive calls:

@Test
public void testGetSerialNumberReturnsBiggerNumbersOnFurtherCalls() {
    firstResult = instance.getSerialNumber(tableName);
    long secondResult = instance.getSerialNumber(tableName);
    assertTrue(firstResult < secondResult);
}

If we want to have more informations we can ask the program wrapper for an array of AS400Message objects:

private String buildMessage(final ProgramCall pgm) {
    AS400Message[] messageList = pgm.getMessageList();
    String message = "";
    for (int i = 0; i < messageList.length; i++) {
        AS400Message aS400Message = messageList[i];
        message += aS400Message.getText();
        message += "\r\n";
    }
    return message;
}

The RPGLE program already ensures that no serial can be duplicated, but I thought that adding a little bit of safety wouldn't be too much damage, so the main call is synchronized. The final draft:

import com.ibm.as400.access.AS400;
import com.ibm.as400.access.AS400Message;
import com.ibm.as400.access.AS400PackedDecimal;
import com.ibm.as400.access.AS400Text;
import com.ibm.as400.access.CommandCall;
import com.ibm.as400.access.ProgramCall;
import com.ibm.as400.access.ProgramParameter;
import com.ibm.as400.access.QSYSObjectPathName;
import java.math.BigDecimal;
import java.util.logging.Level;
import java.util.logging.Logger;

public class MyIseriesSerialsService implements SerialsService {

    private final String myLib = "MYLIB";
    private final String myOtherLib = "MYOTHERLIB";
    private final String myPgm = "MYPGM";
    private final String pgmExtension = "PGM";
    private final AS400 system;
    private QSYSObjectPathName pgmName;

    public MyIseriesSerialsService(final AS400 system) {
        this.system = system;
        addLibraries();
        createPathName();
    }

    public long getSerialNumber(final String tableName) {
        synchronized (this) {
            ProgramParameter[] paramList = createInputParameters(tableName);
            ProgramCall pgm = createProgramCall(paramList);

            try {
                pgm.run();
            } catch (Exception ex) {
                Logger.getLogger(MyIseriesSerialsService.class.getName()).
                        log(Level.SEVERE, ex.getMessage(), ex);
                String errorMessage = buildMessage(pgm);
                throw new RuntimeException(errorMessage, ex);
            }

            long result = extractResult(paramList);

            return result;
        }
    }

    private void addLibraries() {
        try {
            CommandCall cc = new CommandCall(system);
            cc.setCommand("ADDLIBLE " + myOtherLib);
            cc.run();
        } catch (Exception ex) {
            Logger.getLogger(MyIseriesSerialsService.class.getName()).
                    log(Level.SEVERE, ex.getMessage, ex);
            throw new RuntimeException("Unable to inizialize service", ex);
        }
    }

    private void createPathName() {
        pgmName = new QSYSObjectPathName(myLib, myPgm, pgmExtension);
    }

    private ProgramParameter[] createInputParameters(final String tableName) {
        ProgramParameter[] paramList = new ProgramParameter[2];
        paramList[0] = createInputParameter(tableName);
        paramList[1] = new ProgramParameter(32);
        return paramList;
    }

    private ProgramParameter createInputParameter(final String tableName) {
        AS400Text textConverter = new AS400Text(10, system);
        byte[] key = textConverter.toBytes(tableName);
        return new ProgramParameter(key);
    }

    private ProgramCall createProgramCall(final ProgramParameter[] paramList) {
        return new ProgramCall(system, pgmName.getPath(), paramList);
    }

    private long extractResult(final ProgramParameter[] paramList) {
        byte[] data = paramList[1].getOutputData();
        AS400PackedDecimal pdconverter = new AS400PackedDecimal(12, 0);
        return ((BigDecimal) pdconverter.toObject(data)).longValue();
    }

    private String buildMessage(final ProgramCall pgm) {
        AS400Message[] messageList = pgm.getMessageList();
        String message = "";
        for (int i = 0; i < messageList.length; i++) {
            AS400Message aS400Message = messageList[i];
            message += aS400Message.getText();
            message += "\r\n";
        }
        return message;
    }
}

And the test class which assisted me in all the small refactorings:

import com.ibm.as400.access.AS400;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import static org.junit.Assert.*;

public class MyIseriesSerialsServiceTest {

    private final String host = "myVeryExpensiveHost";
 private final String username = "myUsername";
    private final String password = "myPassword";
    private final String tableName = "myTable";
    //
    private MyIseriesSerialsService instance;

    @Before
    public void setUp() {
        instance = new MyIseriesSerialsService(createValidSystem());
    }
    private AS400 createValidSystem() {
        return new AS400(host, username, password);
    }

    @Test
    public void testConnection() throws Exception {
        AS400 system = new AS400(host, username, password);
        assertNotNull(system.getRelease());
    }

    @Test(expected = IllegalArgumentException.class)
    public void testWithWrongTableNameShouldThrowException() {
        instance.getSerialNumber("WrongName");
    }

    @Test
    public void testGetSerialNumberReturnsPositiveNumber() throws Exception {
        long result = instance.getSerialNumber(tableName);
        assertTrue(result > 0);
    }

    @Test
    public void testGetSerialNumberReturnsBiggerNumbersOnFurtherCalls() {
        long firstResult = instance.getSerialNumber(tableName);
        long secondResult = instance.getSerialNumber(tableName);
        assertTrue(firstResult < secondResult);
    }
}

As I said, this is but a draft, and could be improved in many ways, e.g. the tests are quite coarse and don't consider all the small things that could go wrong, some of which I discovered with a quick debugging while I was setting up the tests. Calling a system.disconnectAllServices() when you finish would not be bad either. Yet, it is easily readable and hopefully understandable, so I hope this will help everyone to get rid of the extra stack required by the application server when it is not needed (does that ring a bell?).

FTP in Java to an IFS folder

A couple of years ago I wrote about uploading files via FTP to a remote server, ending the post reminding about the Commons Net library. Well, the time has come for me to use it, having to upload a file to an IFS folder.

The naive approach didn't work:

@Test
public void basicTest() throws Exception {
    URL url = new URL("ftp://user:pass@remoteserver/path/to/remote/folder/" + "outputFile.txt");
    URLConnection urlc = url.openConnection();
    OutputStream outputStream = urlc.getOutputStream();

    String text = "oooooo sooooleee miiiiiiiooooooo.....";
    InputStreamReader reader = new InputStreamReader(IOUtils.toInputStream(text));

    int mychar = reader.read();
    while (mychar != -1) {
        outputStream.write(mychar);
        mychar = reader.read();
    }
    reader.close();
    outputStream.close();
}

as it returned the "550-Specified library does not exist or cannot be accessed." error. Too bad. After some investigations we found that before we could access the folder we were interested in we had to issue the "cd /" command. All right, I went for the commons:

@Test
public void commonsTest() throws Exception {

    FTPClient ftp = new FTPClient();
    ftp.connect("remoteserver");
    ftp.login("user", "pass");

    ftp.changeWorkingDirectory("/");
    ftp.changeWorkingDirectory("path/to/remote/folder");

    OutputStream outputStream = ftp.storeFileStream("outputFile.txt");
   
    String text = "oooooo sooooleee miiiiiiiooooooo.....";
    InputStreamReader reader = new InputStreamReader(IOUtils.toInputStream(text));

    int mychar = reader.read();
    while (mychar != -1) {
        outputStream.write(mychar);
        mychar = reader.read();
    }

    reader.close();
    outputStream.close();

    ftp.logout();
    ftp.disconnect();
}

Obviously this code is simplistic and pretends that errors cannot happen, but it's just to grab the sense of it. We create an instance of the FTPClient class and connect to our remote server (after all we are dealing with sockets... does that ring a bell?), then we use the normal FTP commands to reach the folder we need. After getting an OutputStream, obtained with the storeFileStream(String filename) method, we do everything like in the previous example, then log out and disconnect from the server.

Meet Flatworm

I'm OK with flat files, but why should you use a fixed-length record when every row is formed by 1320 chars, most of which are blank? Isn't it an enormous waste of resources?

Anyway, should you need to deal with flat files (it seems like no programmer can keep them at bay... does this ring a bell?) after some investigation I stumbled upon Flatworm, an interesting library that lets you read from a flat file, be it fixed-length or separated by a separator character, and instantiate the appropriate beans. It also supports repeating segments or multi-line records. Nicely enough, it also work the other way around, which it what I was primarily interested in.

All you have to do is provide a descriptor in XML format, sit back and relax. Let's see how it works.

Let's suppose we need to produce a fixed-length file with the following format:

XXvalueOne  valueTwo  

i.e. a fixed record identifier and two fields of 10 chars each.

First of all you have to provide the descriptor:

<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE file-format SYSTEM "http://www.blackbear.com/dtds/flatworm-data-description_1_0.dtd">
 <file-format>
 <converter name="char" class="com.blackbear.flatworm.converters.CoreConverters" method="convertChar" return-type="java.lang.String"/>
 <record name="whatever-record">
     <record-ident>
         <field-ident field-start="0" field-length="2">
             <match-string>XX</match-string>
         </field-ident>
     </record-ident>
     <record-definition>
         <bean name="whatever" class="my.package.Whatever"/>
         <line>
             <record-element length="2"/>
             <record-element length="10" beanref="whatever.propOne" type="char">
                 <conversion-option name="justify" value="left"/>
             </record-element>
             <record-element length="10" beanref="whatever.propTwo" type="char">
                 <conversion-option name="justify" value="left"/>
             </record-element>
         </line>
     </record-definition>
 </record>
</file-format>

Then you can write a simple class that, given an iterator of whatever you have to export, creates a file and populates it:

public class SimpleExporter {

  FileCreator fileCreator;
  Iterator<Whatever> iterator;

  public SimpleExporter(
          Iterator<Whatever> iterator,
          final String configFile,
          final String outputFile)
          throws FlatwormCreatorException {
      this.iterator = iterator;
      InputStream config = Thread.currentThread().
              getContextClassLoader().
              getResourceAsStream(configFile);
      fileCreator = new FileCreator(config, outputFile);
  }

  public void execute() {
      try {
          fileCreator.setRecordSeperator("\r\n");
          fileCreator.open();
          while (iterator.hasNext()) {
              Whatever whatever = iterator.next();
              fileCreator.setBean("whatever", whatever);
              fileCreator.write("whatever-record");
          }
          fileCreator.close();
      } catch (IOException ex) {
          Logger.getLogger(SimpleExporter.class.getName()).log(Level.SEVERE, null, ex);
      } catch (FlatwormCreatorException ex) {
          Logger.getLogger(SimpleExporter.class.getName()).log(Level.SEVERE, null, ex);
      }
  }
}

Exceptions should be managed row by row, but just stay with me for the example, OK?

For the sake of this example the Whatever class is just a POJO, so it's not worth reporting it here. So what have I done? I just created a FileCreator object passing it an InputStream to the descriptor and the path for the output file. That was not hard, was it?

If your bean has inner properties you can simply use a dot notation:

<record-element length="10" beanref="whatever.outerProperty.innerProperty" type="char">

Playing around I had some little tricks to learn: maybe there's a better way, but they work :-) for example I had to write several fields which are not present in my beans. For this I simply added a "filler" property of type String and used it in all such cases, adding a comment in the desctriptor to specify what I was substituting.

Another problem emerged when the properties in my bean were null; to fix this once and for all I simply extended the CoreConverters class adding null-safe operations:

@Override
public String convertChar(String str, Map<String, ConversionOption> options) {
   return super.convertChar(str == null ? "" : str, options);
}

That said, I found the library really useful as it saved me a lot of time.

Varargs for creating lists

I confess I am not so used to varargs yet, but sometimes they can be very useful, e.g. when you want to create a list and punch in some objects in one shot:

List<String> stooges = Arrays.asList("Larry", "Moe", "Curly");

Without the Arrays class you should use the old boring construct:

List<String> stooges = new ArrayList<String>
stooges.add("Larry");
stooges.add("Moe")
stooges.add("Curly");

The former is much better!

More on classloaders

As an update for my post on classloaders, PMD informs me that in J2EE I should use

Thread.currentThread.getContextClassloader()

My bars are green, a manual end-to-end test makes my testers happy, so this will be our preferred syntax from now on.

How to refactor and test a SAAJServlet

Mind you, not a simple SAAJServlet, but an old fashioned legacy transaction script without the scent of a test. Anyway... let's pick up the usual books (Working effectively with legacy code, Refactoring, Refactoring to patterns - just a starting point of course) to keep at hand as a reference and let's start refactoring.

First a word of caution: don't just copy from the books. Study them, try to apply the refactorings in a small and controlled environment, maybe in little katas, and get the basics beyond them, otherwise you'll do more harm than good.

Back to business. We basically have to test the onMessage method, that gets a SOAPMessage and returns another one. Without even looking at the code, one would expect an algorithm like the following one:

• parse the message into a parameter
• pass it to a service that executes whatever must be executed
  
• build a new message based on the input parameter and on the outcome of the previous step

Obviously one should also consider errors management, but let's keep it simple. Let's take a look at the actual code:

@Override
public SOAPMessage onMessage(SOAPMessage message) {
  SOAPMessage result = null;
  Dialogue dialogue = null;
  try {
    SOAPBody body = message.getSOAPBody();
  
    SOAPFactory sFactory = SOAPFactory.newInstance();
    Name bodyName = sFactory.createName("myLocalName", "myPrefix", "myUri");
    Iterator it = body.getChildElements(bodyName);
    if (it.hasNext()) {
      SOAPBodyElement be = (SOAPBodyElement) it.next();
      dialogue = parseBodyElement(be);
      Operation operation = operationFactory.getOperation(dialogue);
      if (operation != null) {
        try {
          operation.execute(dialogue);
        } catch (Throwable th) {
          getServletContext().log(th.getMessage(), th);
          dialogue.setCode(Dialogue.KO);
          dialogue.setMessage("Service unavailable.");
        }
      }
    } else {
      dialogue.setCode(Dialogue.KO);
      dialogue.setMessage("Malformed message.");
    }
  } catch (Throwable th) {
    getServletContext().log(th.getMessage(), th);
    dialogue.setCode(Dialogue.KO);
    dialogue.setMessage("Service unavailable.");
  } finally {
    try {
      MessageResponseGenerator mrg = new MessageResponseGenerator();
      result = mrg.generateResponseMessage(dialogue);
    } catch (Throwable th) {
      result = null;
      th.printStackTrace();
    }
  }
  return result;
}

Now, though PMD does not complain too much about the structure of the method - just a few dataflow anomaly analysis, null assignments and catching throwables - I smell (more than) a rat.

Starting from the beginning, the first things that strucks me is the different level of abstractions of the code: you have both a low level implementation that extracts a SOAPBodyElement to be parsed and a call to an Operation that encapsulates the requested service (the Command pattern), and this is not good. There are also too many nested ifs and try-catch blocks to my liking.

There is a separated object that creates the response message, and this is good. Why on earth are we missing an object or at least a method that parses the message in the first place?

Why do we ask a factory for an Operation passing in a Dialogue and then we have to pass the same parameter back to the operation we get back from the factory? couln't the operation hold a reference to the dialogue? Would it be better? we shall see later.

And, obviously, all the references are hard-coded...

So far, we have just conducted a small code review. But what's the point in the first place? We have to introduce a new Operation object (maybe extending an existing one, and please note the alliteration). Nothing strange so far, but the resulting object for this operation should contain some more fields than the existing one. Still nothing terrible, but as you might have noticed currently the Dialogue object works as a collecting parameter through all the method, while we would need two different objects (or modify the initial parsing method in such a way to have an instance of the right Dialogue subclass, or introduce a composition in the Dialogue class, or explore a thousand other possibilities).

By the way, it is mandatory that we do not change the interface for the service, which means that the incoming and outcoming messages for the existing operations cannot change.

The fact is that before we start refactoring we have to guarantee that. Of course we could start refactoring right away, but, as you might have noticed if this is not the first post of mine you happen to read, I find it "just a little bit" risky.

So the answer to the first question, i.e. how to refactor a SAAJ servlet, is, at least in my mind, simple: write enough tests to describe the current behaviour before you do anything else. I know: easier said than done.

That's tricky: how can you do that? The class we have to test is a servlet, so we have four options:

• use manual testing: we already have this in place, but each single change would require at least a dozen tests, making the whole process too slow (and not reliably repeatable, as it is based on people's good will)
• mock all the objects provided by the servlet container needed by the servlet: quite a lot of work, and I'm not going down that way (even because I'm listening to Starway to Heaven and I don't wont to spoil the happy sensation)
• deploy the application to a servlet container and use in-container testing, possibly with a framework like Cactus: this introduces an unwanted complexity and I'll avoid it too even if it would be tempting, as the servlet is deployed in a proprietary framework in which you can only access the database if you actually start the webapp (don't ask, you don't want to know). Why? because I think it would not force me to separate responsibilities as much as the next approach.
• use a library such as HttpUnit, which contains ServletUnit. Being this a simulated container I am sure I would never have access to the database given the current situation, so I guess I'll have to work my way to a cleaner design.

Back to the servlet: there are cases in which the method returns a message without going all the way down to the database, so as a first step I'll exercise these and keep manual testing for end-to-end situations.

The very first test consists in producing a message with a wrong format: I should get a SOAPMessage with the code property set as KO and an error message of "Malformed message". Or should I?

@Test
public void testOnMessageWithMalformedMessage() throws IOException, SAXException {
System.out.println("testOnMessageWithMalformedMessage");

InputStream resourceAsStream = this.getClass().
  getClassLoader().
  getResourceAsStream(
  "path/to/my/message.xml");

ServletRunner sr = new ServletRunner();
sr.registerServlet("myServlet", MyServlet.class.getName());

ServletUnitClient sc = sr.newClient();
WebRequest request = new PostMethodWebRequest(
  "http://localhost:8084/myServlet",
  resourceAsStream,
  "text/xml");

WebResponse response = sc.getResponse(request);
assertNotNull("No response received", response);
assertEquals("OK", response.getResponseMessage());
assertTrue(response.getText().contains("KO"));
assertTrue(response.getText().contains("Malformed message."));
}

These steps are detailed in the ServletUnit documentation. Before we run the test, remember we're not going to change anything yet.

Too optimistic? actually I was. Running the tests gives me the following error:

Error on HTTP request: 500 javax.servlet.ServletException: SAAJ Post failed null.

Looks like the MessageResponseGenerator cannot handle null parameters very well. I admit I knew I had been too optimistic about the error message (how could I trust the generator had the same message the servlet used and that I was expecting?), but at least I hoped in a valid SOAPMessage... But after all isn't the NullPointerException the most widespread error in Java code?

It's a long way to go... remeber that before we change the current behaviour we have to describe it, so the way is even longer. Yet, the first step has been made.

Quarreling with classloaders

I admit that most of the quarreling, if not all of it, came from my not too deep knowledge of the subject. Anyway, at least I've learned something more.

Now that I know how to change the endpoint defined in a WSDL I was trying to put this knowledge to good use. We have a project that encapsulates the access to various web services, and we release it as a jar file that is normally used in web applications, in particular the one I'm mostly working on in these days. To easily let the administrator of the web application decide whether to use a monitoring application like wsmonitor I prepared a sevices.properties file:

proxyEnabled=true;
proxyAddress=http://...

Being test-infected I wrote a simple explorative test:

@Test
public void testLoadProperties() throws IOException {
 System.out.println("testLoadProperties");
 Properties properties = new Properties();
 InputStream systemResourceAsStream = ClassLoader.getSystemResourceAsStream("path/to/my/services.properties");
 properties.load(systemResourceAsStream);
 assertNotNull(properties.getProperty("proxyEnabled"));
}

Green bar, so I put the code in the service class. Well actually before that I wrote another small (failing) test:

@Test
public void testPropertiesAreCorrectlyInitialized(){
 System.out.println("testPropertiesAreCorrectlyInitialized");
 assertNotNull(MyService.properties.getProperty("proxyEnabled"));
}

After the green bar I switched to the webapp and started it, but the isProxyEnabled method of the service, which simply performs a test on the value of the isProxyEnabled property always returned false. If you are wondering... yes, there are the corresponding tests, which are not reported for the sake of simplicity.

After some researches, and mostly thanks to Dave Lander's contribution to this discussion, I've found the problem and changed my loading instructions:

InputStream is = MyService.class.getClassLoader().getResourceAsStream("path/to/my/services.properties");
properties.load(is);

Funny how simple things are when you know them, aren't they?

How to get a byte array from a String

Just a snippet which willingly overlooks the fine prints about "strange" characters:

String myString = "Oh what a wonderful string!"
byte[] bytes = myString.getBytes();

Joda Time vs java.sql.Date

As Naked Objects does not (yet) support the Joda Time library, I'm using plain old java.sql.Date to model dates in a prototype I'm working on. I'm not used to it anymore, and that's awful. As an example, let's suppose we have to create an entity with two Date properties, say creationDate and dueDate: the former is asked to the Clock class from the app library, and the latter is derived adding two working days to the former one. For the sake of simplicity, let's only consider adding two days, regardless of the fact that they are working days or not (you always can take it as an exercise).

Calendar cal = Clock.getTimeAsCalendar();
Date creationDate = new Date(cal.getTimeInMillis());
cal.add(Calendar.DAY_OF_MONTH, 1);
Date dueDate = new Date(cal.getTimeInMillis());

Now, with Joda Time I would just write

LocalDate creationDate = new LocalDate(Clock.getTime());
LocalDate dueDate = creationDate.plusDays(2);

Does it compare to the traditional Date and Calendar approach? I think it doesn't...Moreover, should Joda Time be supported, we could also ask for a factory method:

LocalDate creationDate = Clock.getLocalDate;

A better programmer?

After writing about it a couple of years ago recently I took the BetterProgrammer test.

It turned out I am not so bad, even if I'm sure there's plenty of room to improve my skills, mostly because the tests also track the time it takes you to submit the solutions; I'm pretty sure my answers were correct, as for each task I produced almost all the unit tests I could think of (before I actually produced my code, that's obvious).

The tests were very interesting and can be used as katas; some of them could also be resolved by brute force, but some knowledge of maths and combinatorics surely helped me a lot in writing faster and more elegant solutions (it hasn't given me a penny yet, but it turns out that getting a University degree with full marks has yielded some results after all).

You can check your results against mine here.

How to change Tomcat memory settings

Apache Tomcat comes out of the box with a default configuration that uses just a small amount of memory (64MB). As you can easily imagine, that could not be enough in many situations. If you are using Tomcat as a service on Windows you can easily punch in some parameters that will ease your life using a graphical interface that can be accessed from the Start menu:


This will display the following form:


To increase the PermGen space, i.e. the memory area in which compiled code is stored, you have to add the parameter -XX:MaxPermSize in the Java Options section (I have used 256MB). Switching our attention to Heap space, Initial memory pool and Maximum memory pool, just like the Thread stack size, have a field of their own.

Of course the same parameters can be applied from the command line, which is what you most likely are using if you run your Tomcat on a different OS... if this is the case I'm sure you don't need me to write the detailed procedure here as you probably know it like the back of your hand ;-)

Where has all the blue gone?

Today I visited the java web page and... I was shocked! I knew something like this would happen, but now it looks kinda weird...


Besides, if you try to reach Sun website it gets even weirder as you land on the official Oracle website...

Checking beans properties with JMock

JMock is a great tool when you have to test how an object interacts with other objects for which you still haven't written a single line of code; all you have to do is to annotate the test class...

@RunWith(JMock.class)

...define a mockery...

private Mockery context = new JUnit4Mockery();

...and finally write a test method in which you define the mock objects you need, specify their behaviour then you execute your code and test the results:

@Test
public void testGetTable() {
   System.out.println("getTable");
   context.setImposteriser(ClassImposteriser.INSTANCE);
   final LookupTableRepository repository = context.mock(LookupTableRepository.class);
   final LookupTable table = context.mock(LookupTable.class);
   final LookupTableFacadeResultFactory factory = context.mock(LookupTableFacadeResultFactory.class);
   final LookupTableFacadeResult expResult = context.mock(LookupTableFacadeResult.class);

   context.checking(new Expectations() {{
           oneOf(repository).getTable(with(equal(tableName)));
           will(returnValue(table));
           oneOf(factory).make(with(equal(table)));
           will(returnValue(expResult));
   }});

   LookupTableFacade instance = new LookupTableFacadeImpl(repository, factory);
   LookupTableFacadeResult result = instance.getTable(tableName);
   assertSame(expResult, result);
}

So far, so good. But what if you need to check that your object interacts with others passing as a parameter another object it has just built?

This is where custom matchers come in: you simply add an expectation that specifies that the parameter you're interested in has one property (or more) with a given value:

context.checking(new Expectations() {{
        Matcher<LookupTable> tableNameMatcher = Matchers.hasProperty("tableName", equal(tableName));
        oneOf(repository).save(with(tableNameMatcher));
        will(returnValue(table));
        oneOf(factory).make(with(equal(table)));
        will(returnValue(expResult));
}});

In this example we expect our instance to ask the repository to save an object whose tableName property has the specified value.

Obviously, the matcher can be combined with others:

Matcher<LookupTableItem> codeMatcher = Matchers.hasProperty("code", equal(code));
Matcher<LookupTableItem> descriptionMatcher = Matchers.hasProperty("description", equal(description));
oneOf(repository).save(with(allOf(codeMatcher, descriptionMatcher)));
will(returnValue(table));

Get rid of modifiers

When you are performing queries you sometimes want to treat modified letters like their corresponding unmodified ones, e.g. è should be treated just like a plain e.

The first algorithm that comes into your mind is probably a long switch of modified characters, which is horribly ugly. The second one could be a map, slightly better but still ugly. Both approaches require quite an amount of work, and I didn't (I still don't) like them.

After investigating a little and asking some friends I was more or less resigned, until Gabriele pointed me to what I was actually looking for: the java.text.Normalizer, that lets you transform an ugly string into a neat one with just a single line of code:

result = Normalizer.normalize(myString, Normalizer.Form.NFD);
return result.replaceAll("\\W", "").toUpperCase();

Now, that's what I call quite good...

Become fluent with fluent interfaces

It's been a while since I first read about fluent interfaces, and of course it's been a while I've been using them too, like every test infected guy.

Yesterday I finally decided to give them a try and implement one for a Builder: implementing a fluent interface is embarassingly easy, as basically all you have to do is create setters that return the builder itself and a method that returns the object you're creating.

To give the simplest example that comes to my mind, if you wanted to create an order you could write someting like

OrderBuilder.createOrder()
   .forCustomer(customer)
   .with(STEAK, 1)
   .with(FRIES, 1)
   .with(BEER, 3)
   .build();

The builder actual code might look something like this:

public class OrderBuilder {

   private final Order order;

   private OrderBuilder() {
      this.order = new Order();
   }

   public static OrderBuilder createOrder() {
      return new OrderBuilder();
   }

   public OrderBuilder forCustomer(Customer customer) {
      order.setCustomer(customer);
      return this;
   }

   // ...similar setters ...

   public Order build() {
      // ...maybe perform some validation
      return order;
   }
}

After playing with my code for a while I discovered that implementing a good fluent interface is not as easy as it might seem at first: you have to carefully think about what you really want to do (program by intention), how to clearly express it, and have your APIs reflect it. Nevertheless I think I'll experiment with them a little, they could be another nice trick in my toolbox.

Does a Tomcat ride on a Mustang?

Not always out of the box... on a couple of servers (and, obviously, in the worst time possibile) we experienced the total impossibility to start the Tomcat service (both the 5.5.17 and the 6.0.18) on a Java SE 6, and all we got was a "[2009-04-21 13:59:37] [924 prunsrv.c] [error] Failed creating java C:\Programmi\Java\jdk1.6.0_11\jre\bin\client\jvm.dll" message.

Googleing around, we only found references to endless updates to the Windows registry (no mickey-taking on the OS please... choosing it has never been a choice, bearing it is already hard enough). We were almost beginning to have an axe to grind with people supposed to solve this problem, but as this typically leads nowhere - and a wonderful scapegoating tournament always begins - we chose to stick to Tiger.

Today I stumbled upon a very short post with the solution to our problems: fiendish and hidden as it may seem, there is a dependency on the msvcr71.dll file that does not work properly. We then just copied it in the System32 directory, changed the target JDK with the configurator and everything ran so smooth that I was almost deluded by the utmost simplicity of it.

The birthday greetings kata

On February the 14th the milano-xpug organized a kata whose subject was proposed by Matteo on his blog to introduce the exagonal architecture.

Unluckily I couldn't attend the meeting in the flesh, so I just decided to try the kata out here on my own. I first started from scratch, thinking it would prove more fun; then, as I was taking quite a different approach - you could also say that a whole different design was emerging - I decided to get back and start from the original code provided and refactor it to try and focus on the architectural issues which were the aim of the exercise.

Even if I think this exercise to be quite simple, I decided to post a trace anyway because someone might still find it useful. You can never say!

The heart of the application is the BirthdayService class, whose responsibilities consist in identifying employees whose birthday falls on a given date and sending them a greeting message.

This is the public method, called by the acceptance test:


I know it looks odd, but I've had enough of escaping characters and trying to format code to make it look nice, so I hope you will forgive me.

Anyway, the sendGreetings method parses an input file, creates employees, checks if they are eligible for a greeting message and delegate the actual sending of the message to a private method:


Seems a great bunch of things for a single class... let's try to sort it out. First I decoupled the service from the file with the employees' names introducing the interface EmployeeRepository, with the method getEmployees, then I wrote a FileSystemEmployeeRepository that implemented it. The repository was then injected into the BirthdayService via constructor by
the acceptance test. At this point the code looked like this:

...
List<Employee> employees = repository.getEmployees();
for (Employee employee : employees) {
 if (employee.isBirthday) {
    //compose message and call method that sends it
 }
}
...

Still I thought that the check on the date of birth could be done by the repository itself, so I changed the interface adding the new findEmployeesBornOn(OurDate) method and changed the code so that it looked like this:

...
List<Employee> employees = 
      repository.findEmployeesBornOn(ourDate);
for (Employee employee : employees) {
 //compose message and call method that sends it
}
...

Maybe not a very big change but I think it was worth it, as now the code is cleaner. Then there was the messaging part. First of all I decided to introduce a new BirthdayGreeting class, that produces a message body, a fixed subject and a recipient starting from an employee; at this point I extracted the interface SimpleMessage that exposes these three strings. The code had become

...
if (employee.isBirthday) {
  SimpleMessage greeting = new BirthdayGreeting(employee);
  sendMessage(smptHost, smptPort, "sender@here.com", greeting);
}
...

Still I had to resolve the dependency from the SMTP, so just like I did for the repository I introduced the MessagingService interface with the sendMessage(String sender, SimpleMessage message) method and injected it into the BirthdayService via constructor from the acceptance test. Some nip and tuck to move the body of the sendMessage method from BirthdayService into a new EmailService, implementer of MessagingService, and voila, the new BirthdayService code:

public void sendGreetings(int month, int day) {
  List<Employee> employees = 
      repository.findEmployeesBornOn(month, day);
  for (Employee employee : employees) {
     BirthdayGreeting greeting = new BirthdayGreeting(employee);
     messagingService.sendMessage("sender@here.com", greeting);
  }
}

I also changed the method parameter from OurDate to month, day. Now we have a simple and decoupled service that can focus on its true responsibilities: ask for all people who should receive a greeting, create a new message and delegate its sending.

One might wonder whether the sender belongs where I put it, or if the service should directly instantiate a BirthdayGreeting with a new rather than getting it in some other way. But maybe that's going too far.

BTW I also decided not to use the Employee.isBirthday() method and I created a value object called IsBirthday, created with two parameters representing month and day, with the boolean method forEmployee(Employee). The code in the repository now looks like this:

public List findEmployeesBornOn(int month, int day) {
   List<Employee> list = new ArrayList<Employee>();
   IsBirthday isBirthday = new IsBirthday(month, day);

   for (Employee employee : employees) {
      if (isBirthday.forEmployee(employee)) {
         list.add(employee);
      }
   }
   return Collections.unmodifiableList(list);
}

That's all folks... maybe I'll manage to post the complete code here, where you can find some other (and more accredited) solutions produced by people who actually were there.

FTP in Java

This one is a reminder as well; the example is not complete but I'm sure that it will be enough to get to the point.

FTP is not directly supported in Java, rather it is disguised behind a URLConnection. Once you've opened the connection you just access the appropriate stream and normally use it:

URL url = new URL("ftp://username:password@your.host/complete/path/to/remote/file");
URLConnection connection = url.openConnection();

To "get" a file you use the input stream:

InputStream is = connection.getInputStream();
InputStreamReader reader = new InputStreamReader(is);

then you create a writer...

FileWriter writer = new FileWriter(new File("complete/path/to/local/file"));

and flush what you read:

int tmp;
while ((tmp = reader.read()) != -1) {
writer.write(tmp);
}

aren't we forgetting anything? oh yes, clean up the room:

reader.close();
writer.close();

The dual "put" operation requires an output stream:

OutputStream os = connection.getOutputStream();

and the core of the transfer loop would simply be

os.write(tmp);

Ok, the read-and-flush code is quite ugly, one would problably wrap the reader in a BufferedReader, append and flush instead of directly writing and stuff, but for version 0.1 it will suffice.

For most advanced needs it is always possible to use the Commons Net.

JDK6 Update 10

The new JDK6 Update 10 is available here. More infos on the FAQ page (thanks to Fabrizio for the news).