CodeSOD: Paging the Bean

Today's an interesting case of code that seems perfectly reasonable at first glance. Sent to us from FinalGuy, this Java code caused no end of problems for him due to its rather unexpected behavior:

public void setCurrentPage(int currentPage) { if ( currentPage < 1 ) { currentPage = 1; } if ( currentPage > maxPages ) { currentPage = maxPages; } this.currentPage = currentPage;}

This code exists in the context of a Java Bean, aka a class that's just meant to hold data, and gets some bonus features for accessing that data. And this setter clearly is meant to do some validation, and this is validation that seems pretty reasonable: you can't be on a page of a result set that comes before the first or comes after the last. It's very much an Object Oriented Programming 101 example of how setters are meant to be used to ensure a valid state of your object.

So where's the WTF?

Well, one of the requirements for a Java Bean is that it needs to support no-argument constructors. This means that maxPages is set to 0, unless they overrode the default no argument constructor (they didn't). If you're configuring the object, this means that you must set maxPages before you can set currentPage, introducing a dependency between two properties in the object. This isn't just a bad practice because it annoys your developers, it also can break different approaches to serializing/deserializing your Beans.

More than that, though, this code implicitly sets boundaries, which means you don't know that it changed your input data. The Pythonic aphorism "explicit is better than implicit" is a good practice in any language. A better option would be to have it throw an exception when you send it invalid values, so at least you know they were wrong. The best option, however, would be to follow the single responsibility principle and have a validator object that verifies a correct object state later, after the object has been entirely initialized.

In the end, this code isn't the eyeball-ravaging-mind-exhausting horror that we usually look for on this site. But I think it's interesting that this relatively tame looking code, the kind of thing that's in pretty much any "Introduction to Object Oriented Programming" text, is so very wrong to write.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!