Friday, June 29, 2007
More on calling domain object methods from JSPs

This is to follow up from yesterday's post about calling domain object methods from JSPs. If you'll recall, there were three methods proposed:

  • controller call: make the call in the controller, and pass the results as extra entries in the model
  • data transfer object: put the logic in a special object that provides no-argument methods for retrieving the data in the JSP page
  • taglib: create a taglib that essentially just allows the JSP to call the method itself
Having thought a bit about this for a day on the back burner, I'm pretty sure that going the taglib route is not quite the appropriate one here, at least for this method call (which was essentially a range query); so I think it comes down to either the controller call or the DTO.

So where do each of these make sense? I think if you are pretty sure that a standard web application is the only way you'll really be viewing these results, then the controller call makes a lot of sense, particularly when there's not a lot of model entries to make. The specific example we had, where we were storing a Post object and a List in the model, still keeps the controller pretty simple.

However, I think if you need to do this more than a handful of times, or if you know you are going to be calling the underlying business logic from multiple frontends (maybe via a SOAP web service to power a remote AJAX web widget), then it makes sense to start migrating towards the DTO solution. I'd actually take it a step further now, and encapsulate the DTO creation in a service:

public class PostController implements Controller {
  private PostDTOService postDTOService; // dependency
  public ModelAndView handleRequest(HttpServletRequest request,
                                    HttpServletResponse response) {
    String postId = request.getParameter("postId");
    PostDTO postDTO = postViewService.getDTO(postId, 0, 8);
    Map model = new HashMap();
    model.put("postDTO", postDTO);
    return new ModelAndView(..., model);
  }
}

public class PostDTOService {
  private PostDAO postDAO; // dependency
  public PostDTO getDTO(String postId, int first, int max) {
    Post post = postDao.getPostById(postId);
    if (post == null) return null;
    List<Comment> comments = post.getCommentsByRange(first, max);
    PostDTO dto = new PostDTO();
    dto.setPost(post);
    dto.setComments(comments);
    return dto;
  }
}

public class PostDTO {
  private Post post;
  private List<Comment> comments;
  public Post getPost() { ... }
  public void setPost(Post p) { ... }
  public List<Comment> getComments() { ... }
  public void setComments(List<Comment> cs) { ... }
}

Now everyone's roles can be pretty succinctly described:

  • controller: gather parameters from HTTP request and pass to service; place returned object in model and invoke view
  • service: makes the middleware logic calls to produce the needed data and instantiates the DTO
  • DTO: just carries the data around
  • JSP/view: renders the data contained in the DTO
So, my question to you is: is it too much trouble to add the service layer and DTO just to keep the controller and JSP simple, especially when there's no reason the controller couldn't do this? I'd say the service/DTO route probably has a more elegant design to it, but is there such a thing as going to extremes here?

Thursday, June 28, 2007
How to call domain object methods from JSP

It seems like the earlier post about super() spawned at least a little (offline) discussion, so here's another comparison of some different ways of approaching a problem. People should definitely feel free to post some comments , lend your opinions, offer up code snippets, etc. Ok, so here's the deal. We have a middleware domain object that has a method with arguments, and we essentially want to call that method from a JSP page. To make this concrete, let's suppose we have some software supporting a blog. Here's the middleware for a blog post:

public class Post {
 public String getTopic() { ... }
 public List<Comment> getCommentsByRange(int first, int max) { ... }
}
So, we let's say we have a web page where we want to show, for example, the first eight comments under the post itself. Probably the JSP page will want to, somehow, essentially call post.getCommentsByRange(0,8).
Approach A: Call from controller, add to model. Our controller will look like this (assuming Spring MVC).
public class PostController implements Controller {
 public ModelAndView handleRequest(...) {
   Map model = new HashMap();
   String postId = request.getParameter("postId");
   Post post = postDao.getPost(postId);
   List<Comment> comments = post.getCommentsByRange(0, 8);
   model.put("post", post);
   model.put("comments", comments);
   return new ModelAndView(..., model);
 }
}
And our jsp page will contain the following snippet:
<c:foreach items="${model['comments']}" var="comment">
  <!-- display a Comment in here -->
</c:forEach>
Pros:
  • No extra classes or taglibs needed.
Cons:
  • Presentation/business logic appears in the controller, which is arguably not its job.
  • Controller gets really messy the more items like this have to get added to the model.

Approach B: Create a data transfer object. This creates a special class whose methods take no arguments:
public class PostDTO {
  private Post post;
  private List<Comment> comments;
  public PostDTO(Post post, int start, int max) {
    this.post = post;
    this.comments = post.getCommentsByRange(start,max);
  }
  public getTopic() { return post.getTopic(); }
  public getComments() { return comments; }
}
Now the controller looks like as below. Notice that there's only one object put into the model, so the controller is very simple.
public class PostController implements Controller {
 public ModelAndView handleRequest(...) {
   Map model = new HashMap();
   String postId = request.getParameter("postId");
   Post post = postDao.getPost(postId);
   PostDTO postDTO = new PostDTO(post, 0, 8);
   model.put("postDTO", postDTO);
   return new ModelAndView(..., model);
 }
}
Finally, the JSP snippet looks like:
<c:foreach items="${model['postDTO'].comments}" var="comment">
  <!-- display a Comment in here -->
</c:forEach>
Pros:
  • Standard JSP.
  • Controller stays simple, and does not contain presentation logic.
Cons:
  • We have to create this extra DTO class which doesn't do anything particularly interesting.

Approach C: Use a taglib I'm not going to show the code here (want to keep to my self-imposed time limit), but essentially, we create a taglib that knows how to call the getCommentsByRange() method of a post. The JSP would look like:
<c:set var="comments" value=""/>
<mytags:postComments post="${model['post']}" start="0" max="8" outvar="comments"/>
<c:foreach items="comments" var="comment">
  <!-- display a Comment in here -->
</c:forEach>
Pros:
  • Controller stays simple, and does not contain presentation logic.
Cons:
  • We have to create a taglib which pretty much exists just to allow the jsp to invoke a method on the domain object.
Exercise for the Reader. Which approach would you use, and why? Does it depend on the situation?

Wednesday, June 27, 2007
Chatty unit tests

Did I mention this blog is going to be a bit stream-of-consciousness, hopping around from topic to topic? Anyway, wanted to talk today about "chatty" unit tests--unit tests which produce logging output when they run. Most of the time this is due to logging statements from the class under test. Now, most of our Java classes use commons-logging in the following fashion:

public class Thing {
  private static final Log logger =
    LogFactory.getLog(Thing.class);
  ...
}
This makes sure there's one logger per class (not per object), which is usually what you want, so that you can control logging (particularly at runtime) on a whole package basis. However, now when you run unit tests against Thing, it might spew out a bunch of log messages (particularly if those messages are at the INFO level, or if you log exceptions to ERROR (you are testing the exception cases in your unit tests, right?)). This not only scrolls a bunch of stuff on the screen you probably don't care about (especially if it's for tests that pass), but it also slows your unit tests down with I/O. So, a simple solution is to code it like this:
public class Thing {
  private static final Log defaultLogger =
    LogFactory.getLog(Thing.class);
  private Log logger;
  public Thing() {
    this.logger = defaultLogger;
  }
  public Thing(Log logger) {
    this.logger = logger;
  }
  ...
}
Now, most of the time your class will behave as before, especially in a Spring/Hibernate world where everything is expected to have a default constructor. However, now in your unit tests you can either:
  public void setUp() {
     Log mockLog = EasyMock.createMock(Log.class);
     Thing thing = new Thing(mockLog);
  }
especially if you want to actually test the logging. Or, if you just want the unit test to "shut up" while you run it:
  public void setUp() {
    Log nullLog = new org.apache.commons.logging.impl.NoOpLog();
    Thing thing = new Thing(nullLog);
  }
Note that if a unit test breaks, you can always come back in to setUp and change the call to instantiate the Thing under test to use the default constructor again (and thus bring back more verbose logging).

Tuesday, June 26, 2007
Calling super() considered harmful?

Had an interesting discussion yesterday about how to solve the following problem:

  • You have a class (in our case, it was a Spring MVC controller) which has some dependencies injected, and which you, being a good defensive programmer, want to check that all the needed dependencies are present.
  • Since this is a Spring app, it's ok to use the InitializingBean's afterPropertiesSet method somewhere in here.
  • We want to derive some subclasses from the base class that introduce additional dependencies that we want checked, but we want to continue checking the superclass's dependencies.

How would you do this?

Option A. Template method. This would look as follows:

public class Parent implements InitializingBean {
  private ParentDep parentDep;
  public void afterPropertiesSet() {
    if (parentDep == null) {
      throw new IllegalStateException();
    }
    checkChildConfig();
  }
  public void checkChildConfig() { }
  ...
}

public class Child extends Parent {
  private ChildDep childDep;
  @Override void checkChildConfig() {
    if (childDep == null) {
      throw new IllegalStateException();
    }
  }
  ...
}

Pros: (a) parent checks parent configs, child checks child configs. (b) parent configs are always checked, unless the child decides to do something like override afterPropertiesSet. (c) no use of super. (d) This is the one Martin Fowler likes.

Cons: (a) parent has to be aware it will be subclassed. (b) this gets considerably awkward with more than one level of inheritance (i.e. what if there's a GrandChild class?): Parent may need to be modified, or Child may have to override the afterPropertiesSet method of the Parent to cover this case.

Option 2. Just use super.

This one looks this way:

 public class Parent implements InitializingBean {
   private ParentDep parentDep;
   public void afterPropertiesSet() {
     checkConfig();
   }
   public void checkConfig() {
     if (parentDep == null) {
       throw new IllegalStateException();
     }
   }
   ...
 }

 public class Child extends Parent {
   private ChildDep childDep;
   @Override public void checkConfig() {
     super.checkConfig();
     if (childDep == null) {
       throw new IllegalStateException();
     }
   }
   ...
 }

Pros: (a) Parent checks parent configs, child checks child configs. (b) Parent doesn't care if it gets subclassed, or what the depth of the inheritance hierarchy is. In particular, Parent probably doesn't have to be modified with the addition of more hierarchy.

Cons: (a) Child has to remember to call super.checkConfig in its own checkConfig; if not, then the parent dependencies won't get checked. (b) Martin Fowler will look on you with disdain for using super.

It strikes me that if I'm going to inherit from a class (particularly from one where I don't have access to the original source code of the Parent, or where the coder of the Parent didn't totally contemplate that it would be subclassed), I'm probably going to end up overriding some of the parent's methods here or there, and using the parent's methods in other places. Why is this any different? Part of successfully subclassing a class would mean not breaking the "contracts" established by the Parent, and super gives you a nice, easy way to say "oh yeah, and check what he was checking too."

Opinions?

Monday, June 25, 2007
Programmable IDEs

Ok, I'll admit it. I still use Emacs as my "IDE". It's what I started programming with when I was learning Unix/C in college, and it's always served my needs well. Nowadays, I pretend to be a Java developer, and Emacs still works fine for me. I've been meaning to try Eclipse for some time now, but have ended up having issues getting it to work on my 64-bit RHEL workstation, and I just haven't had time to make it work yet.

However, what Emacs and Eclipse have in common is that they are both programmable IDEs. Emacs is pretty explicitly programmable with Emacs Lisp (in fact, you pretty much couldn't even do any amount of preference setting or customization without learning Elisp, for quite a long while), and Eclipse lets you code up extensions to customize its behavior. So what's the big deal about your IDE being programmable?

Well, it means that you can start writing programs to save yourself work. I remember sitting with a colleague who was using Eclipse while I was getting back up to speed on Java development, and got a twinge of Eclipse envy when he right-clicked on an instance variable and selected "create setters and getters". I had just been typing those out by hand in Emacs, but a couple of hours later, I had hacked it up in Emacs, and use it all the time:

(defun java-insert-getter (bean-name bean-type)
 "Insert a simple getter method."
 (interactive "sInsert getter for bean: \nsInsert getter for %s of type[]: ")
 (let ((upcase-bean-name (concat (capitalize (substring bean-name 0 1))
                 (substring bean-name 1))))
   (let ((bean-setter (concat "set" upcase-bean-name))
     (bean-getter (concat "get" upcase-bean-name))
     (bean-use-type (if (string= "" bean-type)
                upcase-bean-name
              bean-type)))
     (insert (concat "public " bean-use-type " " bean-getter "() {"))
     (c-indent-command)
     (newline)
     (insert (concat "return this." bean-name ";"))
     (c-indent-command)
     (newline)
     (insert "}")
     (c-indent-command)
     (newline)
     (c-indent-command))))

(defun java-insert-setter (bean-name bean-type)
 "Insert a simple setter method."
 (interactive "sInsert setter for bean: \nsInsert setter for %s of type[]: ")
 (let ((upcase-bean-name (concat (capitalize (substring bean-name 0 1))
                 (substring bean-name 1))))
   (let ((bean-setter (concat "set" upcase-bean-name))
     (bean-getter (concat "get" upcase-bean-name))
     (bean-use-type (if (string= "" bean-type)
                upcase-bean-name
              bean-type)))
     (insert (concat "public void " bean-setter
             "(" bean-use-type " " bean-name ") {"))
     (c-indent-command)
     (newline)
     (insert (concat "this." bean-name " = " bean-name ";"))
     (c-indent-command)
     (newline)
     (insert "}")
     (c-indent-command)
     (newline)
     (c-indent-command))))

(defun java-insert-setter-and-getter (bean-name bean-type)
 "Insert both setter and getter methods."
 (interactive "sInsert setter/getter for bean: \nsInsert setter/getter for %s of type[]: ")
 (java-insert-setter bean-name bean-type)
 (java-insert-getter bean-name bean-type))

(setq java-mode-hook '(lambda ()
             (interactive)
             (local-set-key "\C-c\C-i" 'java-insert-setter-and-getter)
             (local-set-key "\C-c\C-j" 'java-insert-setter)
             (local-set-key "\C-c\C-m" 'comment-region)
             (local-set-key "\M-d" 'java-kill-word)
             (if (and window-system (x-display-color-p))
             (font-lock-mode))
                     (setq indent-tabs-mode t)
             (setq tab-width 4)
             (auto-fill-mode 1)))

So what's the point? Isn't this just reinventing the Eclipse wheel? Perhaps, but now I can control exactly what it does and why. In some later posts, we'll talk about test-driven development (TDD) and why I might want to inject a protected getter where I might normally have just put a setter method for dependency injection. Later, I'll probably modify this to add in some basic Javadocs.

But here's the main point: get into the habit of automating your best practices. As we learn and share what those best practices are, we should be building tools / macros / scripts that codify those best practices, to free us up for the algorithm, object model, and application design that's the meat of what we do.

Sunday, June 24, 2007
Code Craftsmanship

Writing software is as much a craft as furniture making. Someone can ask for a chair, and different furniture makers will produce different chairs. Hopefully most of them will support your weight when you sit in them, but the sturdiness and aesthetics will vary, as will, most likely, the particular joints, types of wood, etc.

Obviously, writing code is quite similar; there's getting the job done, and then there's getting it done well. Code can be well- or poorly-designed, well-tested or hard to test, well-documented or sparsely commented. This blog is going to contain posts about software craftsmanship, including tips and tricks, philosophical discussions, and code samples. (As a word of warning, some of the code samples will be in Emacs Lisp!).

In the interests of fostering discussion, I'm going to try to tend towards shorter, more frequent posts rather than long-winded essays which spring fully-formed from my keyboard. Ok, so really my attention span just isn't long enough for the longer form.

Welcome.