Tuesday, November 25, 2008

If you build it they will come

Bill posted a comment to my last blog about seeing long methods and classes with too many responsibilities rather than the other way around. I posted a reply, but I though the reply was worth a short blog.

I like to keep a close eye on primitive instance variables, including Strings. When they start to look at all interesting, I make a class out of them. By interesting I mean that code starts to care about more than just the value. Perhaps decisions are being made, or they are used to influence other instance variables. Perhaps a groups of instance variables are palling around together. Those are all signs of a missing object. Even if it starts out as just a data object with a constructor and getters. Objects are like the Field of Dreams, if you build them, behavior will come.


Bill said...

That's a perfect methodology--I really like it. Even the field of dreams comment is very accurate, I'll probably steal it.

I've been doing the same thing for the past couple years and one really interesting thing that pops out is that collections are virtually always interesting (with code and other variables that directly relate to them).

This implies that you should almost never be passing around naked collections. Even as they get a bit more language support; generics only adds the ability to know what a collection should hold, but not the ability to add code to enforce contracts or a place to add general business logic (what good is data without business logic?).

Although I wouldn't call it an absolute rule yet, since I've started using naked collections as "bad code smell", encapsulating them has always improved my code.

Curtis Cooley said...

Thanks Bill. I totally missed collections as a missing object smell. Thinking back to code I read, the code with encapsulated collections always was easier to work with. The logic to handle the collection is in the collection not in the various clients that use the collection.

Bill said...

This note sort of relates to something I've been considering lately.

There are a handful of features, even in Java, That seem to me--well from a high level conceptual view they are a great idea, but when put into use, the way they tend to be used causes a good deal of bad design.

The most obvious: checked exceptions. Does that even need discussion? Same with finalize...

Anonymous inner classes: I like them and use them, but I can't tell you how many times I've seen GUI code where 3 AICs are virtually identical except for the value of a single constant. The programmers should have recognized and refactored into a real class if they hadn't been brought up on the "Fact" that you always use AICs to handle events.

I worry about this pattern being even more pervasive if (when) they add closures (which can't really be made OO as far as I can tell)

Generics--Makes you feel safer passing around collections, however when your collections are encapsulated the benefit of Generics is severely limited.

Non-final member variables with access above private. Why? Is this ever really necessary?

I think there are one or two more, but they are evading me right now.

andrewsw said...

Is there a non-trivial case you've seen where this idea of naked collections as a bad smell fails?

IOW, is there a point of going too far in the direction of encapsulating naked collections?

Just curious from a newb's perspective...

Bill said...

Andrew, I understand what you are saying, but I can't say that I've ever had a case where encapsulating a collection didn't clarify my intent, improve usability and allow some good OO refactors.

At first it was just a pattern I noticed, but after seeing how well it worked I started always wrapping them (often along with a few other related variables and code) and it's never failed me.

Note, I'm not saying wrap every collection in it's own class--I'm just suggesting that when you find yourself passing a raw collection into a method of another class, think about what your collection really represents, think about anything you need to pass along with that collection, and think about the code that will manipulate it.

All those things are probably the core of a nice small class.