Best way to access and display bean object data?

[noodle_variation_187-X]

I have read about the recommended model 2 architecture, but I am not sure if my implementation really reflects this architecture, so I am going to display some code here.
In brief, I have a jsp that sends and receives the requested data. The controller servlet intercepts the requests and decides what to to. The data model consists of some bean objects that are stored in an array. Upon request, a certain array element is selected and then displayed. My approach works, but this doesn't mean that it's good.

Here is the basic code:

index.jsp

<%!
   // Initialisation
   public void jspInit() {
    BeanGenerator.generateBeans();
		
	}

	} 
%>


<form action="Controller" method="get">
	    <input type="submit" value="1" name="btn1"/>
		<input type="submit" value="2" name="btn2"/>
		<input type="submit" value="3" name="btn3"/>		
</form>

Attribute 1:
    <jsp:expression>BeanGenerator.getSelectedObject().getAttb1() </jsp:expression> 
    <br></br>
Attribute 2:
    <jsp:expression>BeanGenerator.getSelectedObject().getAttb1()</jsp:expression> 
    <br></br>

Controller

protected void doGet(HttpServletRequest request,
			HttpServletResponse response) throws ServletException, IOException {
				
		String target = "index.jsp";

		if (request.getParameter("btn1") != null) {
			BeanGenerator.setSelectedObject(BeanGenerator.getItemById(0));		
		}
	       else if (request.getParameter("btn2") != null) {
			BeanGenerator.setSelectedObject(BeanGenerator.getItemById(1));
		}
               else {
			BeanGenerator.setSelectedObject(BeanGenerator.getItemById(2));
		}
			
		RequestDispatcher dis = request.getRequestDispatcher(target);
	       dis.forward(request, response);
	}

BeanGenerator

public static ArrayList<Item> itemList;

public static Item getItemById(int id){	
		return itemList.get(id);		
	}

// generates bean objects and saves them in itemList
public static void generateBeans() {

// ..............		
	}

Is a Vector maybe a better choice to save the bean objects than an ArrayList, because it's threadsafe?
When and where should I generate the bean objects? The bean objects are static, they don't change during runtime, so it would make sense to generate them only once at application startup and not upon every user request.

I would be very happy if someone criticised this approach.

[Tolls]

That's full of red flags to me.
Why all the static stuff?
Especially the concept of setSelectedX() in something that appears not to be threadsafe, and which will likely change during a single call.

User A presses button 1, User B presses button 2. Both requests hit the server at the same time. It's pot luck as to what happens then and what each user sees.

If the Bean Generator is simply a data store, then you should get the data out of it and store that selected bean in the request.
Get rid of the initialisation on the JSP as it's utterly pointless.
I would turn the generator into a singleton, which would then be initialised when it is constructed.

[noodle_variation_187-X]

The way the beans are created and stored is basically ok? But if they shouldn't be created upon application start, when else should they be created? Because the bean objects are the same for all users, it makes sense to create them only once.

As far as I understood it should work like this:
1. The request with some parameters is sent to the controller.
2. Depending upon the request parameter a certain bean is selected from the array/vector
and then copied and wrapped into the request object.
3. The request object is received and the bean attributes are read and displayed.

Right so far? I'll post the improved version later.

[Tolls]

If it's a singleton then the beans (whatever they may be, as I have no idea, but presume they are constant data of some sort) will be created in the singleton constructor, as I said.
Note that these beans can be read only, as if they change then you will need to synchronise. And I would then question what you were attempting.

The rest is fine, so long as you are getting these things from a singleton.

[noodle_variation_187-X]

Thanks. Ok, then I have one instance of BeanGenerator, but where is the getInstance() method called? If it's called in the doGet() method of the Controller each user would get his own copy of the data in a separate thread. Am I right?

The data source for the bean objects is a relatively small XML file.

I have read that jspDestroy() should be used to clean up and release resources and jspInit() to acquire resources, that's why I don't understand why I shouldn't use it. What could I use instead?

My application is actually quite simple:
It should generate bean objects from an XML file and then display some data and images for every data set in the XML.

[Tolls]

It's called whenever you need to use the BeanGenerator instance.
Since it's a singleton then there will only be one for the app, not one per person.

I think you misudnerstand what 'resources' are. It's used when talking about things external to the Java app that you are using, eg files, streams, database connections. Even then, though, these are usually not kept open. You open and read a file when you need to, then close it. Same with database connections. So, again, I've never seen an app that uses any of these JSP extras.

You have no resources to worry about. Your XML file will be opened, read, and closed within the constructor of the singleton.

[noodle_variation_187-X]

Thank you. I have read that there shouldn't be any member variables in a servlet, but if the data is shared among all requests, member variables can be used. In my case the data is static and all requests get data from a common data set, but every single request can receive different data, so this wouldn't be a good idea.

Here is my updated BeanManager:

public class BeanManager {
		
	public static Vector<Country> countryList;
	private BeanManager(){}
	
	 private static class BeanManagerSingleton
	   {
	     private final static BeanManager INSTANCE = new BeanManager();
	   }
	 
	 public static BeanManager getInstance()
	   {
	     return BeanManagerSingleton.INSTANCE;
	   }
	
	public Object clone() throws CloneNotSupportedException {
        throw new CloneNotSupportedException();
      }
	
	public void generateBeans(String filePath){
		
		......................
		}
	
	public Country getCountryById(int id){	
		return (Country) countryList.get(id);
	}
	
}

And here is the updated Controller:

public class Controller extends HttpServlet {
	
	
	private static final long serialVersionUID = 1L;
	
	protected void doGet(HttpServletRequest request,
			HttpServletResponse response) throws ServletException, IOException {

		String path = "C:\\ECLIPSE_WORKSPACE\\TestProject\\xml\\countries.xml";
                BeanManager.getInstance().generateBeans("path");    		
		String target = "index.jsp";

		if (request.getParameter("1") != null) {
			Country c = new Country();	
			c = BeanManager.getInstance().getCountryById(0); 
			request.setAttribute("CountryObj",c);
		
		} else if (request.getParameter("btn2") != null) {
			.............................
		} else {
			.............................
		}
			
		RequestDispatcher dis = request.getRequestDispatcher(target);
	       dis.forward(request, response);

	}
	
}

Afterwards, I want to read the data in the JSP:

<%
Country c = (Country)request.getAttribute("CountryObj");
%>
Name : <%= c.getName() %>

<!-- An image should be displayed  -->
<img src="${pageContext.request.contextPath}/ImageLoader?imageName=myImage" />

Hope it's not too bad.

[Tolls]

Thank you. I have read that there shouldn't be any member variables in a servlet, but if the data is shared among all requests, member variables can be used. In my case the data is static and all requests get data from a common data set, but every single request can receive different data, so this wouldn't be a good idea.

In general it's not a good idea to have any member attributes in a servlet. Yes, it's possible to do it and make it threadsafe, but it's really easy to get it wrong. And, to be honest, it's not necessary.

Your updated code is much better, and safer.

One thing:

public class BeanManager {
   private static class BeanManagerSingleton
   {
      private final static BeanManager INSTANCE = new BeanManager();
   }
...
}

That's not normally necessary, a plain:

private static class BeanManagerSingleton = new BeanManager();

has the same effect.

Oh (and just noticed). The countryList should be private and non-static. It's an attribute of the manager now that the manager is a singleton.

Otherwise, that's all correct, and threadsafe.

[noodle_variation_187-X]

Basically it's all working now. Thanks for your corrections :-)


I have still some questions:

How can I test on my local machine if my application is really threadsafe or is any application that receives its data solely from the request object (like ${requestScope.DataObj.name} ) automatically threadsafe?


What about this approach:
Best Practice: Using HttpServlet init method

Could this also work with a singleton or is the init() method here mandatory?

If I understand this right the singleton pattern and the init() method are two different approaches to deal with the same problem, but a singleton should be preferred if possible.

[Tolls]

The request is only used (unless you do something strange) on a single thread, so no problem there.
That IBM article only really covers your situation if loading the data takes a long time, in which case you might use an init() that simple calls BeanManager.getInstance() and does nothing else, to kick start it. However I would simply leave that up to the first time a request comes in, which has much the same effect. In other words, it's not worth the trouble.

I also just noticed a mistake in my previous post, which should say:

public class BeanManager {
   private final static BeanManager INSTANCE = new BeanManager();
}

rather than the wierdness I put up there as the way it ought to be...

[noodle_variation_187-X]

Thanks.

I have one last question: Is it possible to set a relative path to the xml file? I searched a lot, but it seems not to be possible, because the application runs in a servlet container

[Tolls]

Yes you can.
You'd need to check what counts as the root for your application (easy enough to do, just File f = new File("randomname.txt"); System.out.println(f.getAbsolutePath());).
I always have to do that sort of thing as I can never remember exactly where in the webapp the root sits.

[noodle_variation_187-X]

Thanks.

When I enter File f = new File("randomname.txt"); Eclipse complains about a wrong argument: "Remove argument to match file."

The reason might be that there are two file classes: import java.io.File and org.apache.tomcat.jni.File

The last one is the one that Eclipse automatically imported.

[Tolls]

Should have told Eclipse to import the other one.
It's basic IO in order to find out the path.

[noodle_variation_187-X]

It works, but it returns the file path of my Eclipse installation, not the global path to my xml-file.

[noodle_variation_187-X]

The hardcoded path that works is: "C:\\ECLIPSE_WORKSPACE\\TestApp\\xml\\data.xml ";

Then I extracted the absolute file path that way:

String relativePath = "/xml/data.xml";
String absolutePath = getServletContext().getRealPath(relativePath);

That worked, but I received a path that is not like the path that I have hardcoded:
C:\ECLIPSE_WORKSPACE\.metadata\.plugins\org.eclips e.wst.server.core\tmp0\wtpwebapps\TestApp\xml\data .xml

[Tolls]

That's because Eclipse deploys apps to that plugin temp directory.
But you didn't need that, since I could have told you that the servlet context path was the root of the webapp.
I thought you wanted to know what the root was for the app in the case of a File (I/O) object.

[noodle_variation_187-X]

My problem is that I have to set an absolute path to the XML file that should be parsed. A relative path seems not to work.

[Tolls]

Why?
Why can that XML not be deployed with your app, in the webapp?
An absolute path seems wrong in lots of ways.

[noodle_variation_187-X]

I think the problem is only caused due to Eclipse deployment. Maybe I should manually copy the XML file into the right directory.