Archive for August, 2007

Java best practices 3 – Eating Exceptions and Mixing JSTL with JSF

26 August 2007

Today we arrived at the third installment about best practices in Java. This time I will be talking about the well known, but often sinned against practice of eating up exceptions. Next to that we will be looking at some cases where mixing JSTL and JSF might not be the best way to go.  

Eating up exceptions; continuing with invalid data

Handling exceptions in Java (and to be fair, in most other languages) is a rather difficult topic. One can try to handle the exception directly (e.g. trying a fall back server after a TimeOutException is thrown when accessing the primary server), but this is certainly not always possible. Confronted with their inability to handle an exception, some programmers choose to just ignore the exception and carry on as if nothing has ever happened. This practice is also known as “eating the exception”. In code it looks like this:

try {
   fooObject.doSomething();
}
catch ( Exception e ) {
   // do nothing
}

The thing is that something bad -has- happened and pretending it hasn’t doesn’t magically make it go away. Compare this with ignoring that little red light which tells you you’re almost out of gas. You can drive on pretending all is fine, but inevitably you’ll find yourself stranded at some desolate place, wishing you hadn’t been so careless.

A major problem with eating up exceptions is that most likely your application is in an invalid state afterwards. Continuing on only drags your problems along until ultimately something crashes anyway.

Such a crash however may occur at a completely different location and at a much later moment, making if very difficult to find out the true cause.

Besides a downright crash, another risk you’ll run is that invalid data might be persisted somewhere or that operations will be carried out with missing data. Suppose you’re the programmer that ate the exception that occurred when a user registered itself and your code allowed an order to take place without having anyone to bill. Chances are high more than a few people won’t be too happy with you.

The least, the very least you can do is log the exception when it happens so you’ll have at least some clue where to look if ‘mysterious’ things happen. Better yet, throw the exception upwards if you can’t handle it. Ultimately it might reach the user in some form (e.g. through an error page). It’s true, users don’t like error messages, but they like a system that tells them something went well when in fact it didn’t even less.

Mixing JSTL and JSF for common cases

For starters, don’t get me wrong. Using both JSTL and JSF on the same page is not a bad practice by itself. It used to be unsupported in the separate JSF 1.1 release, but starting from the version that came with Java EE 5 (JSF 1.2) mixing JSTL and JSF is explicitly supported.

The fact that you can mix them however tends to lure some programmers into using JSTL in ways for which there are better or cleaner JSF alternatives. Two common cases where this happens is for conditional rendering using the c:if tag and building a table from a collection using the c:foreach tag.

Use of the c:if tag in JSF is rarely required as most components make use of a rendered property. If you need to render multiple components conditionally, just wrap them in a h:panelGroup and set the rendered property on that one.

Likewise, use of the c:foreach tag just to build a table can often easily be replaced by the h:dataTable tag. Practically, the results are comparable. Both iterate over a List, array, etc and in effect execute their body’s content. In code they look similar too:

JSTL/JSF:

<c:forEach items="${newsItems.newsItems}" var="newsItem">
	<h:outputText value="#{newsItem.date}" styleClass="textItalic" />
	<h:outputText value="#{newsItem.item}" styleClass="paragraph" escape="false" />
</c:forEach>

JSF:

<h:dataTable value="#{newsItems.newsItems}" var="newsItem">				
	<h:column>				
		<h:outputText value="#{newsItem.date}" styleClass="textItalic" />
		<h:outputText value="#{newsItem.item}" styleClass="paragraph" escape="false" />
	</h:column>
</h:dataTable>


However, not only does an h:dataTable based component look more at place in a JSF page, there are also important technical differences. The JSTL taghandlers aren’t components. They serve to build the component tree and subsequently disappear. For some programmatic processing of the component tree this may pose a problem. Another technical difference is that an iterating JSF component doesn’t in fact create an individual component for each iteration. Instead, its children are typically created only once and are evaluated each iteration. This is in stark contrast with the JSTL foreach tag. Without being a ‘managing’ parent and without explicit knowledge about JSF, the c:foreach can do nothing more than add a new component instance to the tree for each iteration. This may or may not be the end of the world, but you should be aware of this difference.

One advantage of the c:foreach approach may be the fact that it allows you to render markup without using the HTML table element. This is often considered an advantage when creating renderings that are in fact not tables. The limited set of standard components in JSF don’t provide support for this, but the 3rd party library Tomahawk contains a t:dataList component that can be used for exactly this.

A third variant, which by itself isn’t technically bad but just looks out of place, is the usage of JSTL for rendering a table in a page that otherwise consists of JSF components:

JSF/JSTL:

<f:view locale="#{locale.locale}">	
	<h:form >

	<%-- Other JSF components here --%>

		<c:forEach items="${newsItems.newsItems}" var="newsItem">
			<br /><i><c:out value="${newsItem.date}"/></i>
			<br /><c:out value="${newsItem.item}" escapeXml="false"/>
		</c:forEach>

	</h:form>
</f:view>

So the moral of the story is. Yes, you can mix JSTL and JSF (1.2), but don’t do it if there are JSF specific solutions available. Only use JSTL if you’re absolutely sure that the rendering you wish to create really needs it and be aware of any technical consequences.

Continue with the next installment here.

Arjan Tijms 

 

Java best practices 2 – Explicit cases

15 August 2007

This is the second installment of my discussion about various bad practices in Java that I encountered during my work. As outlined in the first installment, this entry will be about “Not structuring different cases explicitly”.

After the first installment some readers wondered why the discussion is called “best practices”, while I actually talk about “bad practices”. The idea here is that recognizing these bad practices helps you in avoiding them and doing the opposite, which is a good practice 😉 

Not structuring different cases explicitly

A particularly nasty bad practice is when programmers don’t structure different cases in their code simply as, well… different cases. Oftentimes this bad practice is introduced into a software system whenever an extension is made to existing code.

We all know the deal; we’ve created a nice and simple Servlet that only takes an ID of something and does some work with that in a clean and straightforward way. Inevitably however a boss or customer comes along, asking for an addition to be made. Now how do you handle this?

A beginning or perhaps less talented developer tends to just keep adding parameters to the URL calling the Servlet, sorting the now implicit cases out as the code progresses. At first this may seem reasonable, but it very soon becomes a total maintenance nightmare. Bug fixing becomes hard (which set of parameters belongs together?) and refactoring becomes near impossible if you need to unravel the tightly knitted fabric of 20 or more possible lines of execution, just to find out what cases the code actually handles. After some given threshold is reached even the original programmer is unable to make any changes at all to the code and development grinds to a halt.

If you ever come to work somewhere and a ‘senior’ developer tells you some piece of code can’t be touched since “it’s too dangerous to make changes”, it’s often because of exactly this bad practice.

To give you some idea of what this would look like in practice, take a look at the following code example. Let’s suppose a Servlet can be called using a URL with the following parameters:

“ownerID, customerID, productID, salesDescription, changeText and changeID”

Now suppose the code handling these would look something like this:

processor.customerID = customerID;
if (ownerID != null) {
   store.setOwnerID( ownerID );
   // lots of other code ...
   int foo = processor.getFoo(); // introduce an intermediate variable
   // again lots of other code ...
   if (changeID != null && changeText == null && customerID > 1) {
       store.setCustomerRegularID( customerID );
       // lots of other code ...
       if (productID != null) {
 
       }
   }
   else if (salesDescription != null && foo != changeID) {
        // again lots of code here
   }
   store.setFoo(foo);
   // More and more code
}
// Lots of other code again ...
if ( productID != null && changeID != null ) {
   // ...
}
// etc etc etc

This already looks pretty bad, but now suppose the “lots of other code” comment is actually replaced with lots of other code. It shouldn’t require too much imagination to understand that it becomes ‘rather difficult’ then to decipher what the code is doing.

The problem here is clear; every ‘command’ given to the code is implicitly expressed through a complex combination of overlapping parameters. Which combination of parameters relates to which command is extremely hard to grasp just by looking at the code. The above code may actually do relatively straightforward things such as “update product description” or “update customer description” but we just can’t see that when looking at the code.

The solution to this problem is equally straightforward; simply adopt the command pattern (described by the GOF in the most excellent book Design Patterns, Element of Reusable Object-Oriented Software). Using this pattern, the above code would look more like this:

  1. switch (command.cmd) {
  2.  
  3.    case updateProductDescription:
  4.       handleUpdateProductDescription(command.params);
  5.       break;
  6.  
  7.    case updateCustomerDescription:
  8.       handleUpdateCustomerDescription(command.params);
  9.       break;
  10.  
  11.    // other cases
  12. }

A similar approach can be used at the URL level. Simply introduce one extra parameter called “cmd” and clearly document the meaning of the rest of the parameters depending on the value of that “cmd” parameter. E.g. compare:

http://example.com/foo?productID=4&description=some_description

with

http://example.com/foo?cmd=updateProductDescription&productID=4&description=some_description

This example may look trivial, but imagine 10 URLs with each a different combination of the parameters mentioned earlier. You’ll appreciate the cmd parameter pretty soon. Please note though that in object oriented frameworks like JSF we rarely need to construct URLs manually like this.

It may be hard to believe, but there’s actually an even more hideous form of the “Not structuring different cases explicitly” bad practice; Variable name re-using. This can actually be a bad practice by itself, but it most often shows up in combination with the former. Variable name re-using is often introduced into a software system when the number of parameters and conditionals in the code has already reached a certain threshold due to the usage of the implicit cases as described above. At this point the developer in question thinks he’s being clever and ‘abstracts’ a number of (partly) common cases by reusing existing variables to hold wildly different things. Of course, this only creates an even bigger mess.

E.g. imagine the first code fragment above starting with this:

  1. if (customerID != null) {
  2.    ownerID = customerID;
  3. }
  4.  
  5. if (changeText != null) {
  6.    ownerID = productID;
  7. }
  8.  
  9. if (salesDescription != null && changeText != null) {
  10.    customerID = ownerID;
  11.    ownerID = salesDescription;
  12. }
  13.  
  14. // Rest of the code as given in the first fragment here

Seems totally insane? The code fragment above is in fact a ‘simplified’ version of live code that I actually encountered during code auditing.

Well, that’s it for today. In the next installment we’ll be talking about “eating up exceptions” and “mixing JSTL and JSF for common cases”.

Arjan Tijms

 

Java best practices

11 August 2007

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:

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. Continue with the next installment here

Arjan Tijms 

css.php best counter