views:

1415

answers:

7

Hi, I've been told that the use of scriptlets (<%= ... %>) in my JSP pages isn't such a great idea.

Can someone with a bit more java/jsp experience please give me some pointers as to how to change this code so its more 'best practice', whatever that may be?

This JSP is actually my sitemesh main decorator page. Basically my web design has a tab strip and a submenu, and i wish to somehow highlight the current tab and show the correct submenu by looking at the current request URI.

<%@ taglib uri="http://www.opensymphony.com/sitemesh/decorator" prefix="decorator" %>

<html>
<head>
  <title>My Events - <decorator:title /></title>
  <link href="<%= request.getContextPath() %>/assets/styles.css" rel="stylesheet" type="text/css" />
</head>
<body>

<div class="tabs">
  <a 
    <%= request.getRequestURI().contains("/events/") ? "class='selected'" : "" %>
    href='<%= request.getContextPath() %>/events/Listing.action'>Events</a>
  <a 
    <%= request.getRequestURI().contains("/people/") ? "class='selected'" : "" %>
    href='<%= request.getContextPath() %>/people/Listing.action'>People</a>
</div>

<div class="submenu">
  <% if(request.getRequestURI().contains("/events/")) { %>
    <a href="Listing.action">List of Events</a>
    |<a href="New.action">New Event</a>
  <% } %>
  <% if(request.getRequestURI().contains("/people/")) { %>
    <a href="Listing.action">List of People</a>
    |<a href="New.action">New Person</a>
  <% } %>  
  &nbsp;
</div>

<div class="body">
  <decorator:body />
</div>

</body>
</html>

Thanks all

+1  A: 

You'll need to use some web framework. Or at least some convenient taglib. Or a templating enginge like FreeMarker.

Ad frameworks:

If you like JSP way of coding, then I'd suggest Struts 2.

<s:if test="%{false}">
    <div>Will Not Be Executed</div>
</s:if>
<s:elseif test="%{true}">
    <div>Will Be Executed</div>
</s:elseif>
<s:else>
    <div>Will Not Be Executed</div>
</s:else>

Then there's component-oriented JSF.

If you like OOP and coding everything in Java, try Wicket.

Ondra Žižka
I'm using struts2 if that helps.
Chris
Well now combine it with the tag libraries like JSTL suggested by Vincent, and you're free of scriptlets. See here: http://struts.apache.org/2.1.8.1/docs/tag-reference.html and there you'll find, amongst others, http://struts.apache.org/2.1.8.1/docs/text.html
Ondra Žižka
+3  A: 

You may want to start by using tag libraries. You can use the standard tag library JSTL to do most of the common things that you need scriplets for. There are many other richer tag libraries that are used like in the struts2 framework or from apache.

e.g.

  <c:if test="${your condition}">
       Your Content
  </c:if>

would replace your if statements.

Vincent Ramdhanie
+3  A: 

The preferred alternative to scriptlets is the JSTL expression language; here's a good overview. You'll need to add the taglib like so:

<%@ taglib uri='http://java.sun.com/jsp/jstl/core' prefix='c' %>

As an example, JSTL provides a bunch of implicit objects that give you the stuff you need; the one you want is pageContext.request.

So you can replace <%request.getRequestURI%> with ${pageContext.request.requestURI}.

You can do conditionals using <c:if> tags.

JacobM
JSTL != Expression Language. JSTL is a standard taglib http://java.sun.com/products/jsp/jstl/1.1/docs/tlddocs/ Expression Language are those `${}` things http://java.sun.com/javaee/5/docs/tutorial/doc/bnahq.html
BalusC
For future reference: I've had more luck with 'http://java.sun.com/jsp/jstl/core' in my taglib uri (instead of http://java.sun.com/jstl/core)
Chris
The /jsp/-less uri is indeed part of the a decade old JSTL 1.0. Don't use it.
BalusC
+2  A: 

As an aside, is <%= request.getContextPath() %> an acceptable use of scriptlets that isn't frowned on so much?

This may be an unpopular opinion, but if all you do are simple conditionals and text insertions, I cannot find much fault in the use of scriptlets. (Note the if)

I'd probably use JSTL and the expression language, but mostly because it can be less typing, and IDE support may be better (but a good JSP IDE can also find missing closing brackets and stuff like that).

But fundamentally (as in "keep logic out of templates") I fail to see any difference between

<% if(request.getRequestURI().contains("/events/")) { %>

and

${fn:contains(pageContext.request.requestURI, '/events/') 
Thilo
Ok great! I thought i was the only one with this opinion!
Chris
This is exactly my view too.
Ree
+6  A: 

I think it helps more if you see with your own eyes that it can actually be done entirely without scriptlets.

Here's a 1 on 1 rewrite with help of under each JSTL (just drop jstl-1.2.jar in /WEB-INF/lib) core and functions taglib:

<%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %>
<%@ taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn" %>

<html>
<head>
  <title>My Events - <decorator:title /></title>
  <link href="${pageContext.request.contextPath}/assets/styles.css" rel="stylesheet" type="text/css" />
</head>
<body>

<div class="tabs">
  <a 
    ${fn:contains(pageContext.request.requestURI, '/events/') ? 'class="selected"' : ''}
    href="${pageContext.request.contextPath}/events/Listing.action">Events</a>
  <a 
    ${fn:contains(pageContext.request.requestURI, '/people/') ? 'class="selected"' : ''}
    href="${pageContext.request.contextPath}/people/Listing.action">People</a>
</div>

<div class="submenu">
  <c:if test="${fn:contains(pageContext.request.requestURI, '/events/')}">
    <a href="Listing.action">List of Events</a>
    |<a href="New.action">New Event</a>
  </c:if>
  <c:if test="${fn:contains(pageContext.request.requestURI, '/people/')}">
    <a href="Listing.action">List of People</a>
    |<a href="New.action">New Person</a>
  </c:if>
  &nbsp;
</div>

Here's a more optimized rewrite, note that I used c:set to "cache" expression results for reuse and that I use HTML <base> tag to avoid putting the context path in every link (just make all relative URL's in your webpage relative to it --without the leading slash!):

<%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %>
<%@ taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn" %>

<c:set var="isEvents" value="${fn:contains(pageContext.request.requestURI, '/events/')}" />
<c:set var="isPeople" value="${fn:contains(pageContext.request.requestURI, '/people/')}" />

<html>
<head>
  <title>My Events - <decorator:title /></title>
  <base href="${pageContext.request.contextPath}">
  <link href="assets/styles.css" rel="stylesheet" type="text/css" />
</head>
<body>

<div class="tabs">
  <a ${isEvents ? 'class="selected"' : ''} href="events/Listing.action">Events</a>
  <a ${isPeople ? 'class="selected"' : ''} href="people/Listing.action">People</a>
</div>

<div class="submenu">
  <c:if test="${isEvents}">
    <a href="Listing.action">List of Events</a>|<a href="New.action">New Event</a>
  </c:if>
  <c:if test="${isPeople}">
    <a href="Listing.action">List of People</a>|<a href="New.action">New Person</a>
  </c:if>
  &nbsp;
</div>

It can actually be optimized more if you collect all those "hardcoded" values like events and people and link texts in a Map in the application scope and use under each the JSTL <c:forEach> to display the tabs.

As to your actual question, you can disable scriptlets (and get runtime errors about using it) by adding the following entry in webapp's web.xml. It may help to spot overseen scriptlets.

<jsp-config>
    <jsp-property-group>
        <url-pattern>*.jsp</url-pattern>
        <scripting-invalid>true</scripting-invalid>
    </jsp-property-group>
</jsp-config>

To learn more about EL, check the Java EE tutorial part II chapter 5. Implicit EL objects, such as ${pageContext} are described here. To learn more about JSTL, check the Java EE tutorial part II chapter 7. Note that JSTL and EL are two separate things. JSTL is a standard taglib and EL just enables to access backend data programmatically. Although it is normally used in taglibs like JSTL, it can also be used standalone in template text.

BalusC
Awesome answer! I owe you a beer if you're ever in sydney! Thanks for being so helpful and polite.
Chris
You're welcome. I'll mind the beer.
BalusC
If you compare the "before" and "after" templates, you'll see that they're pretty much the same. I don't see how the latter is automatically better than the former (besides looking a tiny bit cleaner).
Ree
+3  A: 

This isn't a direct answer to your question (and there are already several good ones, so I won't try to add to it), but you did mention:

Can someone with a bit more java/jsp experience please give me some pointers as to how to change this code so its more 'best practice', whatever that may be?

In my opinion, best practice, with regards to JSP, is that it should be used strictly as a templating engine, and no more (i.e., no business logic in there). Using JSTL, as many pointed out, definitely helps you get there, but even with JSTL, it's easy to do to much in a JSP.

I personally like to follow the rules laid out in Enforcing Strict Model-View Separation in Templating Engines by the Terence Parr when developing in JSP. The paper mentions the purpose of templating engines (separating model and view), and characteristics of a good templating engine. It takes a good look at JSP and points out ways it's not a good templating engine. Not surprisingly, JSP is basically too powerful and allows developers to do too much. I strongly recommend reading this paper, and it'll help you restrict yourself to the "good" parts of JSP.

If you read only one section in that paper, read chapter 7, which includes the following rules:

  1. the view cannot modify the model either by directly altering model data objects or by invoking methods on the model that cause side-effects. That is, a template can access data from the model and invoke methods, but such references must be side-effect free. This rule arises partially because data references must be order-insensitive. See Section 7.1.
  2. the view cannot perform computations upon dependent data values because the computations may change in the future and they should be neatly encapsulated in the model in any case. For example, the view cannot compute book sale prices as “$price*.90”. To be independent of the model, the view cannot make assumptions about the meaning of data.
  3. the view cannot compare dependent data values, but can test the properties of data such as presence/absence or length of a multi-valued data value. Tests like $bloodPressure<120 must be moved to the model as doctors like to keep reduc- ing the max systolic pressure on us. Expressions in the view must be replaced with a test for presence of a value simulat- ing a boolean such as $bloodPressureOk!=null Template output can be conditional on model data and com- putations, the conditional just has to be computed in the model. Even simple tests that make negative values red should be computed in the model; the right level of abstraction is usu- ally something higher level such as “department x is losing money.”
  4. the view cannot make data type assumptions. Some type assumptions are obvious when the view assumes a data value is a date, for example, but more subtle type assumptions ap- pear: If a template assumes $userID is an integer, the pro- grammer cannot change this value to be a non-numeric in the model without breaking the template. This rule forbids array indexing such as colorCode[$topic] and $name[$ID] The view further cannot call methods with arguments be- cause (statically or dynamically) there is an assumed argu- ment type, unless one could guarantee the model method merely treated them as objects. Besides graphics designers are not programmers; expecting them to invoke methods and know what to pass is unrealistic.
  5. data from the model must not contain display or layout information. The model cannot pass any display informa- tion to the view disguised as data values. This includes not passing the name of a template to apply to other data values.

Incidentally, Terence has created his own templating engine called String Template which supposedly does a really good job of enforcing these rules. I have no personal experience with it, but would love to check it out on my next project.

Jack Leow
A: 

Scriptlets aren't the worst thing in the world. An important consideration is to think about who is going to be maintaining the code. If its web designers who don't have much Java experience, you are probably better off going with tag libraries. However, if Java developers are doing the maintainance, it may be easier on them to go with scriptlets.

If you end up using a tag library and JSTL, you are expecting the maintainer to also learn the tag library and know JSTL. Some developers will be fine with this as it is a skill they want or already have, but for some developers who only have to deal with JSPs every few months or so, it can be lot less painful to work with clearly written scriptlets written in nice, familiar Java.

Spike Williams