Archive for the ‘ide’ Category

The * stupidest things I’ve done in my programming job

Saturday, February 7th, 2009

I’m not ashamed of those sins any more, so here you go :)

1. ORM

Stupidity
Building my own Object Relational Mapping framework.
Consequence
Project is a mess after 2 years of maintenance with hardcore hacks to bypass my own ORM and call custom SQL queries.
What should I have done
Use hibernate, iBATIS, Cayenne or something similar.

2. EAV

Stupidity
Using an Entity-Attribute-Value model database schema design.
Consequence
Non scalable solution and total impossibility to run any useful queries on the database level.
What should I have done
Use an ordinary normalized database schema design.

3. Database Access

Stupidity
Synchronize (serialize) database access using one shared connection.
Consequence
Zero scalability. Very slow response times when more than 10 users where using the application.
What should I have done
Don’t do that and use a connection pool such as c3p0 and use a “new” (reused) connection returned from the pool for every request/response cycle.

4. IDE

Stupidity
Avoided learning and using an Integrated development environment.
Consequence
Inability to build test and deploy the application quickly and generally do anything useful.
What should I have done
Get familiar with an IDE. NetBeans, eclipse etc.

5. Transactions

Stupidity
Not using them.
Consequence
Corrupt data in an application involving e-shop like functionality.
What should I have done
Use database transactions. When in MySQL use InnoDB.

6. Prepared Statements

Stupidity
Using Statements, string concatenation and naive character escaping to assemble my own “safe” queries.
Consequence
SQL Injections possible in my application. I managed to login using ‘or 1=1;delete from users;– and alter the database state in a very nasty way.
What should I have done
Use Prepared Statements which correctly assemble and escape the query properly depending on the JDBC driver used.

7. Business Logic

Stupidity
Doing it in the template (JSP).
Consequence
Messy non maintainable application.
What should I have done
Do it in an MVC style with servlets or with a Front Controller. Even better by using an existing open source MVC framework such as Struts, Spring MVC etc.

Of course, all the bad choices above have probably made me a better programmer.

Test First (#2)

Wednesday, August 1st, 2007

You want to create a helper class to analyse an HttpServletRequest and provide you with helper methods which determine the type of request. You basically care about GET and POST request methods. More specifically you want to be able to tell whether a POST is Multipart (usually containing file uploads).

Without losing a second, you create the class and make it compile. Right now you only care about the API of the class, and how will it be used from the client side of view. The class is immutable.

import javax.servlet.http.HttpServletRequest;

public class HttpRequestAnalyser {

  private final boolean get;
  private final boolean post;
  private final boolean multipart;

  public HttpRequestAnalyser(HttpServletRequest request) {
    // do nothing of value (yet)
    this.get = false;
    this.post = false;
    this.multipart = false;
  }

  public boolean isGet() {
    return get;
  }

  public boolean isPost() {
    return post;
  }

  public boolean isMultipart() {
    return multipart;
  }

}

Press F9, it compiles (NetBeans).
No matter what kind of request we construct this class with, it will never give us a true answer on whether the method was GET, POST or POST-MULTIPART. That’s because the implementation is the absolute minimum we need in order to define our API and have something that compiles.

Press CTRL+SHIFT+U to create the following empty test.

import junit.framework.*;

public class HttpRequestAnalyserTest extends TestCase {
  
  public HttpRequestAnalyserTest(String testName) {
    super(testName);
  }

  public void setUp() {
  }

  public void testIsGet() {
  }

  public void testIsPost() {
  }

  public void testIsMultipart() {
  }
  
}

Press SHIFT+F6, the test passes because there are no assertions yet.

The point is to create good tests for our HttpRequestAnalyser class. The tests will fail, and then we’ll try to make them pass by doing real work in our class, which is the System Under Test.

In order to test our class we will need instances of HttpServletRequest. It’s not (easily) possible to create such instances, as this is an interface, and usually the servlet container generates concrete implementations. We don’t want to incorporate a servlet container in our tests. What we are going to do is mock an HttpServletRequest, as if it was coming over http from a client.

In order to mock such a request we could provide our own implementation, possibly in an inner static class, inside our test and we’d have to implement all methods.

Another way is to create a mock object which will implement HttpServletRequest and program it with canned answers for our class. There exist many frameworks out there for mocking and we’ll use mocquer.

We need to add the servlet-api.jar and the mocquer dependencies on our project’s test libraries.

In order to fake HTTP request data, we need to know how these look like. Firefox and many other http header monitor tools out there help us gather the following interesting parts of the requests:

GET method

GET /example.html HTTP/1.1
Host: www.domain.com

POST method

POST /example.html HTTP/1.1
Host: www.domain.com
Content-Type: application/x-www-form-urlencoded

POST method with file upload

POST /example.html HTTP/1.1
Host: www.domain.com
Content-Type: multipart/form-data; boundary=...

The test now becomes:

import javax.servlet.http.HttpServletRequest;
import junit.framework.*;
import org.jingle.mocquer.MockControl;

public class HttpRequestAnalyserTest extends TestCase {
  
  public HttpRequestAnalyserTest(String testName) {
    super(testName);
  }
  
  private MockControl requestControl;
  private HttpServletRequest request;
  
  public void setUp() {
    requestControl = MockControl.createControl(HttpServletRequest.class);
    request = (HttpServletRequest)requestControl.getMock();
  }
  
  public void testIsGet() {
    request.getMethod();
    requestControl.setReturnValue("GET");
    requestControl.replay();
    HttpRequestAnalyser analyser = new HttpRequestAnalyser(request);

    assertTrue(analyser.isGet());
    assertFalse(analyser.isPost());
    assertFalse(analyser.isMultipart());
  }
  
  public void testIsPost() {
    request.getMethod();
    requestControl.setReturnValue("POST");
    request.getHeader("Content-Type");
    requestControl.setReturnValue("application/x-www-form-urlencoded");
    requestControl.replay();
    HttpRequestAnalyser analyser = new HttpRequestAnalyser(request);

    assertFalse(analyser.isGet());
    assertTrue(analyser.isPost());
    assertFalse(analyser.isMultipart());
  }
  
  public void testIsMultipart() {
    request.getMethod();
    requestControl.setReturnValue("POST");
    request.getHeader("Content-Type");
    requestControl.setReturnValue("multipart/form-data; boundary=...");
    requestControl.replay();
    HttpRequestAnalyser analyser = new HttpRequestAnalyser(request);

    assertFalse(analyser.isGet());
    assertTrue(analyser.isPost());
    assertTrue(analyser.isMultipart());
  }
  
}

We’ve provided method call expectations, and canned answers on the mock. The tests fail.
We fill in the HttpRequestAnalyser constructor implementation which now becomes:

import javax.servlet.http.HttpServletRequest;

public class HttpRequestAnalyser {
  
  private final boolean get;
  private final boolean post;
  private final boolean multipart;
  
  public HttpRequestAnalyser(HttpServletRequest request) {
    this.get = request.getMethod().equals("GET");
    this.post = request.getMethod().equals("POST");
    this.multipart =
        request.getHeader("Content-Type").startsWith("multipart/");
  }
  
  public boolean isGet() {
    return get;
  }
  
  public boolean isPost() {
    return post;
  }
  
  public boolean isMultipart() {
    return multipart;
  }
  
}

Post and multipart tests pass. The get test fails, because we haven’t set an expectation for getHeader(“Content-Type”) to be called. That is because the GET Http method doesn’t have such a header.
The multipart check should only happen if the request has been recognised to be a post:

import javax.servlet.http.HttpServletRequest;

public class HttpRequestAnalyser {
  
  private final boolean get;
  private final boolean post;
  private final boolean multipart;
  
  public HttpRequestAnalyser(HttpServletRequest request) {
    this.get = request.getMethod().equals("GET");
    this.post = request.getMethod().equals("POST");
    this.multipart = post ?
      request.getHeader("Content-Type").startsWith("multipart/") :
      false;
  }
  
  public boolean isGet() {
    return get;
  }
  
  public boolean isPost() {
    return post;
  }
  
  public boolean isMultipart() {
    return multipart;
  }
  
}

Tests pass, and the class is ready to rock! Happy testing (before coding).

Good Javadoc use

Sunday, July 29th, 2007

Javadoc is a tool that you have in your %JAVA_HOME%\bin (Windows) or $JAVA_HOME/bin (UNIX) directory. It is being used to extract API documentation from your Java source code, into HTML.

Most IDEs fully support Javadoc. They assist the developer in writing, displaying and generating the Javadoc documentation. Javadoc is part of the development lifecycle. Good and up-to-date Javadoc is a sign of a healthy project.

API Users

The worst thing an API user can do is to ignore Javadoc. Having an easily accessible, local copy of the Javadoc for the library you are working with (Spring, Hibernate, Lucene etc) is the only way to work efficiently. Vast amounts of time have been put into compiling quality documentation for these libraries. Not using it will only slow you down.

Development Teams

One of the worst Netbeans feature is that it allows you to collapse all comments and even set this as the default behavior when opening a source file. Team members, working in the same piece of code, can no longer see the documentation written by other developers.
I actually had a developer asking me about what one of my methods did. For a moment I was confused and asked him whether I had written poor documentation. He then told me that he didn’t know that there was any Javadoc for that method. I don’t (want to) know whether Javadoc hiding is a feature of Netbeans or is a plugin, but that was definitely one of the most ineffective behaviors I’ve seen in a development team.

API designers

Writing good Javadoc involves many things which are documented by SUN. The general idea is that you don’t want to expose the implementation of the method through the documentation. API users will not care how you do it, but only what (and maybe why) you do.
So the following comment is bad:

/** 
 * This method uses StringBuffer to merge the name with surname and return
 * the person's full name
 */
 public String getFullName() {

A better version is:

/** 
 * Returns the full name of the Person. Will return "Nick Cave" if
 * firstname is "Nick" and lastname is "Cave".
 *
 * @return      the full name of the Person
 */
 public String getFullName() {

Conclusion

We (Java developers) are very lucky that documentation is so tightly integrated with the language. Not using it though, takes all this goodness away.

Learn to use the debugger

Monday, April 30th, 2007

Your favourite IDE has a powerful debugger which you can use to debug your programs. If you are new to programming, chances are that you are aware of it’s existence, but never use it.

The problem

Here is an example method, which has a problem. (It actually does nothing of value, but demonstrates the problem case). The method doSomething is called with a parameter, but the expected result is not returned.

public Box doSomething(Box foo, Box bar) {
  Box temp = foo.cloneMe();
  if (bar!=null) {
    Box newBox = new Box(bar);
    if (newBox!=null) {
      temp = doSomethingElse(foo);
      if (foo==null) {
        temp = bar;
      }
    }
  }
  return temp;
}

Here you can see the most frequent (ab)use of System.out.println statements.

public Box doSomething(Box foo, Box bar) {
  System.out.println("1");
  Box temp = foo.cloneMe();
  if (bar!=null) {
    System.out.println("2");
    Box newBox = new Box(bar);
    if (newBox!=null) {
      System.out.println("3");
      temp = doSomethingElse(foo);
      if (foo==null) {
        System.out.println("4");
        temp = bar;
      }
    }
  }
  return temp;
}

The developer’s intent is to trace which if-statements execute, so as to find the bug. If 1 2 3 is displayed in the console, the developer knows that foo==null evaluated to false.

An “enhancement” of this method is to add variables of interest in those System.out.println statements.

public Box doSomething(Box foo, Box bar) {
  System.out.println("1: " + foo + " " + bar);
  Box temp = foo.cloneMe();
  if (bar!=null) {
    System.out.println("2: " + temp);
    Box newBox = new Box(bar);
    if (newBox!=null) {
      System.out.println("3: " + newBox);
      temp = doSomethingElse(foo);
      if (foo==null) {
        System.out.println("4: " + temp);
        temp = bar;
      }
    }
  }
  return temp;
}

This is one of the most crude ways to debug a program. Unfortunately it’s quite common between junior developers. Note that if logging needs to be performed (for monitoring or historical purposes) a proper logging framework has to be used.

Things get interesting when the developer forgets to delete those System.out.println statements. The application is deployed, in a servlet container which hosts more applications, featuring code “debugged” in this way.
It’s not rare to see catalina.out logs which look like this:

INFO: Find registry server-registry.xml at classpath resource
20 Απρ 2007 10:23:24 μμ org.apache.catalina.startup.Catalina start
INFO: Server startup in 14063 ms
20 Απρ 2007 10:23:24 μμ org.apache.catalina.core.StandardContext reload
INFO: Reloading this Context has started
1
2
is null
3
copying file pic_01.jpg->temp/pic_01.jpg
copying file pic_02.jpg->temp/pic_02.jpg
copying file pic_03.jpg->temp/pic_03.jpg
copying file pic_06.jpg->temp/pic_06.jpg
1
2
is null
3
true
4
5
6

** BEGIN NESTED EXCEPTION ** 

java.net.ConnectException
MESSAGE: Connection refused: connect

STACKTRACE:

java.net.ConnectException: Connection refused: connect
	at java.net.PlainSocketImpl.socketConnect(Native Method)
	at java.net.PlainSocketImpl.doConnect(PlainSocketImpl.java:333)
...
is null
false
java.lang.NullPointerException
BoxExample$Box@126b249
1
2
is null
3
resultset was null
1
resultset was null
...

The solution

Learn to use your debugger. All you have to do is go to the line you want debugging to start, set a breakpoint (CTRL+F8 in Netbeans) and start the debug process. You will either debug the whole application (F5) or that single file/unit test (CTRL+SHIFT+F5).
You can set watches, see the stacktrace and examine the contents of all the local variables at any time in the program execution. You get orders of magnitude more power, in less time; for free!

Try it out. When you get used to it, you’ll never look back.

Test First (#1)

Saturday, April 28th, 2007

Unit testing not only ensures that the code you write is correct, but also helps you develop your code. This can be achieved by testing unimplemented methods and functionalities first, and then “filling in” the code to satisfy the tests. This is the “test first” rule that your XP coworker will always remind you, at any occasion. (You do have an XP coworker, don’t you? :)

The Requirement

Suppose you have a webapp which displays some data from a database. Some of those texts are long, and you are asked to be able to truncate them at predefined lengths on some particular views. So instead of printing “Lorem ipsum dolor sit” you should be able to fit that in 12 chars and print “Lorem ips…”.

Wait

The first worst thing you could do there would be to stick that logic straight into the view (JSP, velocity or whatever templating engine you use). You will not be able to test that piece of logic, nor easily reuse it in some other project. You will need to do this in a Java class and then find a way to call it from the template.

The second worst thing you could do would be to implement this functionality yourself, as it already exists in commons lang. You should know what APIs exist out there and try to reuse as often as possible. But anyhow, we’ll assume that you want to do it yourself.

Think

You start by thinking of where to place this method and whether it will be a helper (static?) method or part of a full fledged class with state etc. Then you choose a good method name and what parameters it will accept. Think of how you would like to use this method.

Code

You write the method signature:

public static String truncate(String text, int length) {
}

Then fill it’s body to the absolute minimum to make it “compilable”:

public static String truncate(String text, int length) {
  throw new RuntimeException("not implemented");
}

…or…

public static String truncate(String text, int length) {
  return "";
}

And then stop, because that’s all you need to implement for now.

Test

You create a test (hit CTRL+SHIFT+U if you use http://www.netbeans.org/) and create a test for the method truncate

public void testTruncate() {
}

This test passes, because it doesn’t test anything. You make things more interesting by adding some basic assertions of what you expect from this method.

public void testTruncate() {
  assertEquals("Lorem ip...", Demo.truncate("Lorem ipsum dolor sit", 11));
  assertEquals("Lorem ips...", Demo.truncate("Lorem ipsum dolor sit", 12));
  assertEquals("Lorem ipsu...", Demo.truncate("Lorem ipsum dolor sit", 13));
}

You run the test and it fails.
You go back to the source code and implement some logic to satisfy this test.

Code

public static final String truncate(String text, int length) {
  return text.substring(0, length-3) + "...";
}

You run the test and it now passes.

Test

It’s time to test for some corner cases.

public void testTruncate() {
  assertEquals("", Demo.truncate("Lorem", 0));
  assertEquals(".", Demo.truncate("Lorem", 1));
  assertEquals("..", Demo.truncate("Lorem", 2));
  assertEquals("...", Demo.truncate("Lorem", 3));
  assertEquals("Lorem ip...", Demo.truncate("Lorem ipsum dolor sit", 11));
  assertEquals("Lorem ips...", Demo.truncate("Lorem ipsum dolor sit", 12));
  assertEquals("Lorem ipsu...", Demo.truncate("Lorem ipsum dolor sit", 13));
  assertEquals("Lorem ipsum dolor sit", Demo.truncate("Lorem ipsum dolor sit", 21));
}

All new assertions fail with a StringIndexOutOfBoundsException. You write code to satisfy these corner cases.

Code

public static final String truncate(String text, int length) {
  switch(length) {
    case 0: return "";
    case 1: return ".";
    case 2: return "..";
    case 3: return "...";
    default:
      return length<text.length() ?
        text.substring(0, length-3) + "..." :
        text;
  }
}

You run the test and it now passes.

Test

Then you test even more.

public void testTruncate() {
  assertEquals("", Demo.truncate("Lorem", 0));
  assertEquals(".", Demo.truncate("Lorem", 1));
  assertEquals("..", Demo.truncate("Lorem", 2));
  assertEquals("...", Demo.truncate("Lorem", 3));
  assertEquals("Lorem ip...", Demo.truncate("Lorem ipsum dolor sit", 11));
  assertEquals("Lorem ips...", Demo.truncate("Lorem ipsum dolor sit", 12));
  assertEquals("Lorem ipsu...", Demo.truncate("Lorem ipsum dolor sit", 13));
  assertEquals("Lorem ipsum dolor sit", Demo.truncate("Lorem ipsum dolor sit", 21));
  try {
    Demo.truncate("Lorem ipsum dolor sit", -1);
    fail("Should have thrown illegal argument exception");
  } catch (IllegalArgumentException expected) { }
}

This is a standard idiom for testing that an exception should be thrown. If this method is called with a negative length parameter, we’d like an IllegalArgumentException to be thrown. It is not mandatory to test for things like these, but some people like to seal their methods from really bad usage.

Code

public static final String truncate(String text, int length) {
  if (length<0) {
    throw new IllegalArgumentException("Length cannot be negative");
  }
  switch(length) {
    case 0: return "";
    case 1: return ".";
    case 2: return "..";
    case 3: return "...";
    default:
      return length<text.length() ?
        text.substring(0, length-3) + "..." :
        text;
  }
}

You run the test and it now passes. You are done.

Conclusion

This implementation might not be the best in the world, but right now this doesn’t matter. The code runs, and it’s robust. Whenever you feel like, you can refactor it and make it perform better. The test will be there to guide you.

p.s What we’ve omitted when we wrote the signature of this method, was to write Javadoc. It is very important to document your API, and we’ll discuss that in another post.