J-Development

Within Mbuyu, the company I work with, one of the things I’m responsible for is guarding the quality of our code base. This job mainly involves reading through source code and marking dubious constructs and practices. In the past I’ve been doing quite similar things at other locations.

Over time I came across a number of bad practices that seem to be repeated over and over. Many of those originate from people who are just beginning their Java career; new employees, interns etc. Surprisingly, even some more experienced Java developers sometimes sin on these seemingly straightforward rules.

Of course, there is a subjective factor involved here. People actually differ on what is a best practice and what is not. Anyway, without further ado, let’s start with a list of some common bad practices:

• Not using types
• Not validating user input
• Mixing business logic and view code
• Not structuring different cases explicitly
• Eating up exceptions; continuing with invalid data
• Mixing JSTL and JSF for common cases
• Using native arrays instead of ArrayList
• Not using Java 5 features where appropriate
• Coding to a class instead of to an interface
• Selecting ResultSet columns by index, instead of by name
• Needless use of instance variables

Due to the size of the discussion, I shall discuss only the first 3 items of this list today and leave the rest to a follow-up posting.

Not using types

This may sound like a weird bad practice in Java. After all, Java is a strongly typed language, so how can we not be using types when the compiler enforces them? Actually, there are at least two ways around the type system; make everything a String or make everything an Object.

This first option is common in plain JSP programming; data enters the application from request parameters as Strings and developers simply don’t care to convert them to some data type. Instead, business logic methods are written to take Strings and layer upon layer only Strings are passed around. It seems insane to do this, but I’ve actually seen people doing stuff like this for years(!).

The second option is nowadays less common, although it sometimes shows up in JDBC programming; programmers do not exactly know what Java types correspond to SQL types so they just call getObject(); and pass the data along. Probably they’re hoping the next guy will somehow magically know to which type the Object needs to be casted.

Before the introduction of generics in Java 5, the second option was very pervasive in Java code though. At that time there simply was no way to store anything in a collection without resorting to using Object. It couldn’t however really be called a ‘bad practice’ by then, since there was really no sane way to circumvent the problem.

Not validating user input

Not validating user input is one of the most common bad practices I’ve encountered. It gives rise to a whole slew of problems, ranging from SQL injection, to cross-site scripting and excessive exception throwing. For instance, many beginners don’t seem to realize that Javascript validations don’t protect your server from malicious users who can (of course) just send data to your server directly, bypassing any Javascript validation you may have in place.

A more subtle form of this bad practice is when a programmer doesn’t validate if data conforms to business rules right when it enters the system. Instead, such a programmer validates data at some other point in time, perhaps when the data is actually used. Of course, it’s often too late then to correct matters and afterwards the location which allowed for this invalid data is hard or impossible to find.

Mixing business logic and view code

Not separating business- and view code is another frequently encountered practice. It’s a major cause of creating spaghetti from code, which makes bug fixing and applying changes much harder than they should be. This bad practice is especially common for people with a PHP background, where the community more or less seems to encourage this practice (or at least doesn’t discourages it as much as in e.g. the Java or .NET communities).

One major problem with this bad practice is that beginning developers don’t always want to adopt a more sane MVC approach. It’s very much true that the MVC pattern may be overkill for small applications. However, many larger applications tend to be grown out of smaller ones. On top of that, for a new programmer the one or two pages he makes at first often seem to be the entire world, even when the application which is going to include these pages already has perhaps 500 other ones. Seeing the rest of the world is a skill often learned only over time.

For Java EE, an early effort by Sun to gently push the programmer into this MVC model was JSTL. In JSTL the programmer is presented with a number of tags and an expression language (EL) to define the rendering. JSTL contains conditionals, variables, and looping constructs. It should be very clear that these are solely meant to be used for rendering and nothing else. Or isn’t that so clear? A couple of years ago I asked one programmer to stop putting business logic in JSP pages using Java scriptlets and start using JSTL. After putting up some initial resistance, he finally agreed and went back to his work. When I looked through his next CVS commit, I was in for a surprise though. All the business logic was still exactly there in the JSP page, but this guy had simply rewritten the Java scriptlet code into JSTL tags! Needless to say that expressing business logic in the view layer through JSTL is an even worse practice.

Well, I’ll leave it to that today. Stay tuned for the next installment. 

Arjan Tijms 

Dit bericht is geschreven op zaterdag 11 augustus, 2007 om 15:47
en staat in de rubriek Java.