h1

Keep it Legible

December 7, 2006

As usual, Blaine got me thinking; this time about writing legible code, in particular around Jakarta Commons Collections. His take, and I whole-heartedly agree, is that you should first think about how client code has to use your object. Blaine’s initial problem revolved around the ugly code he needed to use Jakarta Collections, as in:

Collection activeAccounts = CollectionUtils
  .select(someAccounts, new Predicate() {
 public boolean evaluate(Object each) {
  return ((Accounteach).isActive();
 }
});

I initially liked his final solution,

// Account class
public static final Predicates BY = new Predicates();

// Class stuff...

static class Predicates {
 public Predicate active() {
  return new Predicate() {
   public boolean evaluate(Object each) {
    return ((Accounteach).isActive();
   }
  };
 }
}


// Client code
Collection activeAccounts = CollectionUtils
  .select(someAccounts, Account.BY.active());

but it eventually dawned on me that the CollectionUtils dependency leaks through to the client. The problem is easily solved with static methods on the Account Class:

public static Collection selectActive(Collection someAccounts) {
 return CollectionUtils
   .select(someAccounts, new Predicate() {
  public boolean evaluate(Object each) {
   return ((Accounteach).isActive();
  }
 });
}


// Client code
Collection active = Account.selectActive(someAccounts);

Even the word, static, makes me shudder but I couldn’t come up with a reason to prefer static inner classes static properties over static methods. I’ll compromise if it means I can hide more implementation from the client.

Advertisements

5 comments

  1. You may, or may not, benefit from not letting the client see the collection at all. Just give them objects, and operations, and whether or not the collection is from java.util, or Jakarta, shouldn’t matter – the client won’t see it.


  2. “you should first think about how client code has to use your object.” == unit tests


  3. Nice post. When creating Jaggregate, I became painfully aware of how odious it can be to instantiate anonymous predicates/functors, especially now that generics are in the mix. I took care to provide some basic predicates/functors and either make them statics or, if they needed to bind to specific arguments, behind static methods that instantiate them as needed. I think it’s proven most helpful.


  4. I’m one of the few (it feels like) that hates using inner classes. I code full classes for each predicate and make them each have a static instance of itself. I feel this makes the code package clearer to the maintainer, who might be just casually browsing the repository.


  5. Ivan, I used to be the same, then someone pointed out that I was making up rather artifical names for those classes.

    If something has no clear name, then it should not be named. If you have classes called things like AccountActivePredicate, that’s dubious – it may be more clear to have that class anonymous in the use site.

    Further, by moving the code outside the use site, you add complexity to the overall scope. Keeping the code in the use site makes only the use site appear more complex.



Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

w

Connecting to %s

%d bloggers like this: