Should I retrieve database record in Struts2 view layer?

Solution 1:

Using Struts2 you won't need to use Scriptlets (<% stuff %>) anymore. They're old, bad, they're business logic injected in view pages, do not use them. You do not need JSTL neither, just using Struts2 tags you can achieve any result.

For a better decoupling and separation of code and concepts, you should have:

1. DAO Layer: it does just the plain queries;
2. BUSINESS Layer: it exposes the DAO Layer results through Service(s), aggregating multiple DAOs calls and performing several business operations when needed;
3. PRESENTATION Layer: The Actions, that in Struts2 acts as the Model; here you call the Service from the Business Layer, to retrieve the objects needed by the JSP;

4. JSP (VIEW Layer): the JSP contains the plain HTML, and accesses the data needed through the Accessors (Getters) of the Action, and eventually any other needed element from the Value Stack (#session, #request, etc).

   In your example, all of this

<% 
   Session session2 = HibernateUtil.getSessionFactory().getCurrentSession();
   Transaction tx = session2.beginTransaction();
   Query q = session2.createQuery("from Subject");
   List subjectList = q.list();
   List levelList = session2.createQuery("from Level").list(); 
%>

should be in DAO/Business Layers, exposed by two function like getSubjectList(); and getLevelList();. Then in your Action you should have something like:

public class YourAction {

    private List<Object> levelList; // private
    private List<Object> subjectList; // private

    public String execute() throws Exception {      
        // Call the service, load data
        levelList = getMyService().getLevelList();
        subjectList = getMyService().getSubjectList();

        // Forwarding to the JSP
        return SUCCESS;
    }

    public List<Object> getLevelList() {
        return levelList;
    }
    public List<Object> getSubjectList() {
        return subjectList;
    }

}

and in your JSP, instead of:

<select name="subject_id">
<%
  for (Object subjectObject : subjectList) {
      subject subject = (Subject) subjectObject;
%>
      <option value="<%=subject.getId()%>"><%=subject.getName()%></option>
<%
  } //end for
%>
</select>

you access the list like (ugly mixed HTML/Struts2 way):

<select name="subject_id">
    <s:iterator value="subjectList">
        <option value="<s:property value="id"/>">
            <s:property value="name"/>
        </option>   
    </s:iterator>
</select>

or, in the case of a Select, with the proper Struts2 UI Select Tag:

<s:select name = "subject_id" 
          list = "subjectList" 
       listKey = "id" 
     listValue = "name" />

If separating all the layers is too difficult at the beginning, flatten the first three levels in the Actions, just to understand how to separata Java (Action) and Struts2 UI Tags (JSP). When understood, you can move the DAO logic to the business layer, preferably into an EJB. When achieved that, split again with more granularity...

The Action will be something LIKE this:

public class YourAction {

    private List<Object> levelList; // private
    private List<Object> subjectList; // private

    public String execute() throws Exception {      
            Session session2 = HibernateUtil.getSessionFactory().getCurrentSession();
            Transaction tx = session2.beginTransaction();
            Query q = session2.createQuery("from Subject");
            subjectList = q.list();
            levelList = session2.createQuery("from Level").list();

        // Forwarding to the JSP
            return SUCCESS;
    }

    public List<Object> getLevelList() {
        return levelList;
    }
    public List<Object> getSubjectList() {
        return subjectList;
    }    
}

About your question on multiple loading of the lists, you can use a cache (better if with a timer) if the list is fixed (it changes one a month for example), or loading it every time, there aren't problems in doing that. Please note that if validation fails, the ValidationInterceptor will forward the request to the JSP mapped in the INPUT type result, without reaching the execute() method, so you should implement Preparable interface from Action and put the loading stuff into prepare() method, execute every time by the PrepareInterceptor

public class YourAction implements Preparable {

    private List<Object> levelList; // private
    private List<Object> subjectList; // private

    public void prepare() throws Exception {
        // Call the service, load data, 
        // every time even if validation fails
        levelList = getMyService().getLevelList();
        subjectList = getMyService().getSubjectList();
    }

    public String execute() throws Exception {      

        // Forwarding to the JSP
        return SUCCESS;
    }

    public List<Object> getLevelList() {
        return levelList;
    }
    public List<Object> getSubjectList() {
        return subjectList;
    }
}

Proceed by steps, the framework is easy and powerful, the web has plenty of examples and StackOverflow provides some great support...

Solution 2:

What you need is a cache. But if the database records are bound to change frequently it's inadvisable.

If however the Query in question is small (I think it is) querying the database shouldn't be a big performance problem.

On another note, looking at your JSP all I see is deprecated and misuse of JSP scriplets.

Since you have added the tag struts 2 I will assume that this is a struts 2 web project. Consider (strongly) using built in struts ui tags for the work done in you scriplets.

Your approach can only be described as using a bunch of dynamos to power a city when you have a nuclear reactor at your disposal.

I suggest you start here : http://struts.apache.org/2.x/docs/home.html

This will give you a proper idea of the framework and it's full capabilities.