Re: how does the Model (Command), in MVC, return objects to a
servlet?
Thufir Hawat wrote:
The code is a bit long:
I'm going to look this over in more detail later, but I have a few
side comments right away. These are minor points that aren't all
going to kill your app, but should be addressed.
package enterprise.servlet;
import javax.servlet.*;
Single-type imports are a best practice, as opposed to import-on-
demand.
import javax.servlet.http.*;
import java.io.*;
import java.net.*;
import java.util.*;
//import java.sql.*;
//import javax.servlet.*;
import enterprise.common.*;
import enterprise.io.*;
/**
* The Base Enterprise Servlet from which all Enterprise Servlet
Applications derive.
* @author Bill Quirk, Jeff Genender
* @version $Revision: 1.41 $
This is perhaps not the best use of the Javadoc @version. Javadocs
are for consumers of the API, those who will not be using version
control. The version those folks care about is the public version,
not the repository version.
*/
public abstract class BaseEnterpriseServlet extends HttpServlet
{
// Servlet constants
private final String METHOD_PREFIX ==
"Method.";
private final String DEFAULT_PREFIX ==
"Default.Method";
//Servlet globals
private MethodList m_methodList =
= null;
Setting a member variable to 'null' explicitly is unnecessary, and
causes the variable to be set to null twice.
Personally, I prefer to identify member variables by the prefix 'this'
and eschew warts like "m_".
public String m_value =
= null;
public String m_default=
= null;
/**
* Runs initialization code the first time the servlet is
* instantiated.
*
* @param config the ServletConfig to work from
*/
public final void init(ServletConfig config) throws ServletExcept=
ion
{
super.init(config);
//Check for the ConfFile parameter on the initialization =
string
String sConf = config.getInitParameter("=
ConfFile");
if(sConf == null)
{
throw new ServletException("ERROR - BaseEnterpris=
eServlet
needs a " +
=
"ConfFile param.\n");
}
try
{
// Parse the config file
parseConfigFile(sConf);
}
catch(Exception e)
{
e.printStackTrace();
throw new ServletException(e.getMessage());
}
}
/**
* Processes the GET and POST method for an HTTP request to thi=
s
servlet
* @param req the HttpServletRequest to work from
* @param res the HttpServletResponse to write back on
*/
public void service(HttpServletRequest req, HttpServletResponse r=
es)
throws ServletException, IOException
You should not override the 'service()' method of 'HttpServlet'.
Instead, override 'doGet()' and 'doPost()'. This could be very bad
for your app.
{
PrintWriter out = null;
//Caching dynamic content is very bad.
//Set the default response header to not cache
res.setContentType("text/html");
res.setHeader("Pragma", "No-cache");
res.setHeader("Cache-Control", "no-cache");
res.setDateHeader("Expires", 0);
This breaks the MVC pattern severely. View code in the servlet code
is a no-no. Use JSPs.
try
{
// Check and see if we have a "method" parameter
// in the query string
String method = req.getParameter("method");
// If we don't, then lets use the default paramet=
er
if (method == null)
{
method = m_default;
}
HTTPMethod methodInstance = null;
// Lets dynamically load the class through a look=
up
// in our method list. Take the method name an=
d lookup
// the cross reference for the full class name. =
If we
// find it, then load it.
Class c = Class.forName( m_methodList.get(metho=
d) );
Use of raw types is a no-no.
// Manually create the instance of the class
methodInstance = (HTTPMethod)c.newInstance();
// Set the request and response paramters in our =
subclassed
// HTTPMethod, so that it may have acces to these=
parameters
methodInstance.registerMethod(req, res);
// Dispatch the request
// This is where the BaseEnterPriseServlet dispat=
ched the
// control to the subclassed HTTPMethod through t=
he execute
// call.
methodInstance.execute();
}
catch (IllegalArgumentException ex)
This isn't really wrong, but runtime exceptions indicate programmer
error, not user-input error. You might consider using a checked
exception for this instead.
{
//If we got here, then the method name was not found =
in the
method list
out = getPrintWriter(res);
out.println("<HTML>");
out.println("Invalid method");
out.println("</HTML>");
out.flush();
Once again you've broken MVC here.
}
catch (Exception ex)
{
ex.printStackTrace();
return;
This 'return' is superfluous.
}
}
/**
* Parses a configuration file.
*
* @param fileName The full path name of the configuration file.
*/
public void parseConfigFile(String fileName)
throws ServletException, IOException
{
ConfFile cf = new ConfFile(fileName);
In the interests of SSCCE, we might eventually want to see the
definitions of these custom types.
try
{
cf.load();
}
catch (IOException ioe)
{
ioe.printStackTrace();
throw new ServletException(ioe.getMessage());
Why do you declare the method to throw 'IOException' then prevent it
from throwing that exception?
}
//Build our list of methods
buildMethodList(fileName);
}
/**
* Build the list of Methods
*
* @param cf The configuration file.
*/
public void buildMethodList(String confFile)
Maybe this method shouldn't be 'public'.
throws ServletException, IOException
{
String defaultMethod = null;
// Get the list of methods.
ConfFile cf = new ConfFile(confFile);
cf.load();
MethodList ml = new MethodList();
Enumeration x = cf.getKeys();
'Enumeration' bad. 'Iterator' good. Unless 'ConfFile' is a
'java.util.Properties', which explains it.
boolean y = x.hasMoreElements();
'hasMoreElements()' bad. 'hasNext()' good. Unless 'ConfFile' is a
'java.util.Properties', which explains it.
for (Enumeration m = cf.getKeys() ; m.hasMoreElements()=
;)
{
String key = (String) m.nextElement();
// If it starts with our Method prefix, then it i=
s a
// method setup paramdeter, so use it.
if (key.startsWith(METHOD_PREFIX))
{
// Get the method's name
String method = key.substring(METHOD_PREFIX=
..length());
if (method.length() > 0)
{
// Get the cross reference class name.
String classpath = cf.getValue(key);
// Add it to the method list
ml.add(method.trim(),classpath.trim());
}
}
// If the parameter is the Default.Method paramet=
er
// then set this up
if (key.equals(DEFAULT_PREFIX))
{
// Get the method that it references.
defaultMethod = cf.getValue(key);
}
}
// Verify that we do indeed have a DefaulMethod parameter
if (defaultMethod == null)
{
throw new ServletException("Default method " + defaul=
tMethod +
" not found in ConfFile.");
}
// Verify that the Default.Method paramter is a valid met=
hod
try
{
ml.get(defaultMethod);
} catch (IllegalArgumentException iae)
{
throw new ServletException("Default method " + defaul=
tMethod +
" is not a valid method.");
}
// Set the default global default value
m_default = defaultMethod;
// Set the global MethodList
m_methodList = ml;
}
/**
*
* Ensures that a print writer is returned, even if and [sic] O=
utputStream
has been created
*
* @returns a PrintWriter for HTTP output
**/
private PrintWriter getPrintWriter(HttpServletResponse res)
throws IOException
{
PrintWriter pw = null;
This initialization to 'null' is superfluous.
try
{
pw = res.getWriter();
}
catch(IllegalStateException ise)
{
pw = new PrintWriter(res.getOutputStream());
}
return pw;
}
}
http://www.amazon.com/Enterprise-Java-Servlets-Jeff-Genender/dp/02017...
Whether that list if methods exists in a configuration file, as above, or
an enum, it's somewhere.
I just saw no way of passing parameters to a factory method in the enum
to return a Command or child of Command, which is what I took you to
mean. Nor do I see what that achieves.
I'll get back to you on this. I don't remember discussing factory
methods so I don't know where that remark comes from.
I just ended up pushing the switch around:
I'll show an example using a 'Map' later. The problem with a 'switch'
is that it hardcodes your choices, so to create new services you must
recompile.
//in the servlet
Result result = new RequestHelper(request).getCommand().execute();
//seems pointless the way I have it
That is a function of the way you have it, not the strength of the
idiom itself.
so that the execute method does a switch on the "method" field of a
Command. However, that method type, or list, is still somewhere. I
don't see how doing an if/then/switch on a field isn't OOP.
It doesn't involve objects or polymorphism, but hardcodes the decision
procedurally.
and it would be nice if there were a ResultQuery or ResultFoo each of
which implement Result, that's fine, however it doesn't seem to matter.
The operative word being "seem".
It seems very different from referring to an ArrayList as a List so that
different kinds of List's can be handled because Result is going to be
passed to the view. Yes, Result must implement a specific interface, s=
o
there could be a Result interface, but why do that?
Because it provides simplicity and maintainability, and the ability to
modify functionality very readily.
--
Lew