My very own MVC framework...

[ankur rathi]

Here are the contents of my web application. Please do comments on anything you feel could be better for example naming convention, xml file structure, package organization, xml parsing etc.


mail-application.xml - This XML file has entry for all the possible actions that may happen in application and classes that handle the actions....

 
<mail-application>
 
<action type="sign-in" action-class="com.ms.action.SignInAction" success-page="welcome.jsp" error-page="signIn.jsp" />
 
<action type="sign-up" action-class="com.ms.action.SignUpAction" success-page="signUp.jsp" />
 
<action type="forgot-password" action-class="com.ms.action.ForgotPasswordAction" success-page="forgotPassword.jsp" />
 
</mail-application>
 

MainController.java - This is main controller. Every request comes to this first and then....

  
 
package com.ms.controller;
 
import java.io.IOException;
 
import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
 
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
 
import com.ms.action.IAction;
import com.ms.actionloader.ActionModelLoader;
import com.ms.actionmodel.Action;
import com.ms.actionmodel.MailApplication;
 
public class MainController extends HttpServlet {
 
MailApplication mailApp = null;
 
public void init(ServletConfig arg0) throws ServletException {
super.init(arg0);
 
ActionModelLoader loader = new ActionModelLoader();
mailApp = loader.loadMailApplication("mail-application.xml");
 
}
 
protected void service(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
super.service(req, resp);
 
String type = req.getParameter("actiontype");
Action action = mailApp.getAction(type);
IAction actionClass = null;
try {
actionClass =
(IAction) Class.forName(action.getActionClass()).newInstance();
} catch (InstantiationException e) {
e.printStackTrace();
} catch (IllegalAccessException e) {
e.printStackTrace();
} catch (ClassNotFoundException e) {
e.printStackTrace();
}
actionClass.doProcess(
req,
resp,
action.getSuccessPage(),
action.getErrorPage());
 
}
}
 

Here are some action classes and one action interface.

  
 
IAction.java
------------
 
package com.ms.action;
 
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
 
public interface IAction {
 
public void doProcess(
HttpServletRequest req,
HttpServletResponse resp,
String successPage,
String errorPage);
 
}
 
 
SignInAction.java
-----------------
 
package com.ms.action;
 
import java.io.IOException;
 
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
 
public class SignInAction implements IAction {
 
public void doProcess(
HttpServletRequest req,
HttpServletResponse resp,
String successPage,
String errorPage) {
 
String username = req.getParameter("username");
String password = req.getParameter("password");
 
RequestDispatcher rd = null;
 
if (username != null && password != null) {
rd = req.getRequestDispatcher(successPage);
} else {
rd = req.getRequestDispatcher(errorPage);
}
try {
rd.forward(req, resp);
} catch (ServletException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
 
}
 
}
 
 
SignUpAction.java
-----------------
 
package com.ms.action;
 
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
 
public class SignUpAction implements IAction {
 
public void doProcess(
HttpServletRequest req,
HttpServletResponse resp,
String successPage,
String errorPage) {
 
}
 
}
 
 
ForgotPasswordAction.java
-------------------------
 
package com.ms.action;
 
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
 
public class ForgotPasswordAction implements IAction {
 
public void doProcess(
HttpServletRequest req,
HttpServletResponse resp,
String successPage,
String errorPage) {
 
}
 
}
 
 
 

Here are objects that store mail-application.xml

  
 
MailApplication.java
--------------------
 
package com.ms.actionmodel;
 
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
 
public class MailApplication {
 
List actionList = new ArrayList();
 
public void addAction(Action action) {
actionList.add(action);
}
 
public Action getAction(String type) {
 
Iterator itr = actionList.iterator();
Action action = null;
while (itr.hasNext()) {
action = (Action) itr.next();
if (action.getType().equals(type)) {
break;
}
}
return action;
}
}
 
 
 
Action.java
-----------
 
package com.ms.actionmodel;
 
public class Action {
 
String type = null;
 
String actionClass = null;
 
String successPage = null;
 
String errorPage = null;
 
public String getActionClass() {
return actionClass;
}
 
public String getErrorPage() {
return errorPage;
}
 
public String getSuccessPage() {
return successPage;
}
 
public String getType() {
return type;
}
 
public void setActionClass(String string) {
actionClass = string;
}
 
public void setErrorPage(String string) {
errorPage = string;
}
 
public void setSuccessPage(String string) {
successPage = string;
}
 
public void setType(String string) {
type = string;
}
 
}
 
 

ActionModelLoader - This class unmarshal mail-application.xml file and stores the content into java objects using JDOM.

 
package com.ms.actionloader;
 
import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.util.Iterator;
 
import org.jdom.Document;
import org.jdom.Element;
import org.jdom.JDOMException;
import org.jdom.input.SAXBuilder;
 
import com.ms.actionmodel.Action;
import com.ms.actionmodel.MailApplication;
 
public class ActionModelLoader {
 
public MailApplication loadMailApplication(String fileName) {
 
MailApplication mailApp = new MailApplication();
Document doc = getDocument(fileName);
Element actionElement = null;
Action action = null;
Iterator itr = doc.getRootElement().getChildren("action").iterator();
while (itr.hasNext()) {
actionElement = (Element) itr.next();
action = new Action();
action.setType(actionElement.getAttributeValue("type"));
action.setActionClass(
actionElement.getAttributeValue("action-class"));
action.setSuccessPage(
actionElement.getAttributeValue("success-page"));
action.setErrorPage(actionElement.getAttributeValue("error-page"));
mailApp.addAction(action);
}
 
return mailApp;
}
 
private Document getDocument(String fileName) {
 
Document doc = null;
ClassLoader loader = Thread.currentThread().getContextClassLoader();
URL resource = loader.getResource(fileName);
File file = new File(resource.getFile());
SAXBuilder builder = new SAXBuilder();
try {
doc = builder.build(file);
} catch (JDOMException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
return doc;
}
 
public static void main(String[] args) {
 
ActionModelLoader aml = new ActionModelLoader();
MailApplication mailApp =
aml.loadMailApplication("mail-application.xml");
 
}
}
 
 

And I have some JSPs also...

Presently, it is giving IllegalStateException in SignInAction class.

java.lang.IllegalStateException: Cannot forward. Response already committed.

But I am more interested in knowing, how can I improve this design and everything....

Please comments.

Thanks a lot for your time.

[ November 23, 2005: Message edited by: rathi ji ]

[Jesper de Jong]

Interesting, but why did you write your own MVC framework when there are already a number of high-quality, well-known, well-tested and debugged frameworks out there like Struts, Spring MVC, WebWork, etc.?

[ankur rathi]

Originally posted by Jesper de Jong:
Interesting, but why did you write your own MVC framework when there are already a number of high-quality, well-known, well-tested and debugged frameworks out there like Struts, Spring MVC, WebWork, etc.?

Just for learning purpose...

[Bear Bibeault]

Interesting, but why did you write your own MVC framework when there are already a number of ...

Not only is it a great learning experience, but sometimes you don't want something with all the bells and whistles of full-blown frameworks that were written to be general purpose and may have a lot of bloat that's not necessary.

I've written my own light-weight framework and it suits my purposes to a tee.

[Ben Souther]

I concur.

Re-inventing the wheel is not a bad thing if the intent is to learn about how wheels are made, or to create a wheel better suited to the task than the ones currently available.

After having built your own, you will be much better suited to the task of evaluating the the other frameworks that are out there.

Just be careful not to end up re-writing Struts or some other framework that you could have downloaded and had up and running in a day.

[Frank Carver]

I have also designed this sort of thing in the past as a learning experience. In your case I have a few questions and suggestions.

1. Is there any particular reason you use an I prefix on your Action interface? I personally find that names work best if they are 100% descriptive of their function, and 0% descriptive of their implementation.

2. defining your own "service" method is usually considered dangerous behaviour. Do you really want your Actions to respond to any old HEAD, PUT or OPTIONS request that appears at the server? Although it's clumsy, the best way is still usually to define one method (with whatever unclaimed name you like) and call it from redefined doGet and doPost methods.

3. I also worry that you have coupled your actions a bit too close to the Servlet API. My preference is to decouple knowledge of servlet API methods from the business logic as it can make unit testing your code immensely easier. This is particularly apparent where you do the request dispatching in the Action.

Given that you define the success and failure URLs externally, I would prefer to process them in the framework rather than the actions. Currently you don't have any sort of return value from your Action methods, so it should be simeple to return (for example) a boolean or enum representing Success or Failure and have the framework do the request dispatching.

For example, somthing like:

protected void doGetPost(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
 
String type = req.getParameter("actiontype");
Action action = mailApp.getAction(type);
IAction actionClass = null;
try {
actionClass =
(IAction) Class.forName(action.getActionClass()).newInstance();
} catch (InstantiationException e) {
e.printStackTrace();
} catch (IllegalAccessException e) {
e.printStackTrace();
} catch (ClassNotFoundException e) {
e.printStackTrace();
}
boolean itWorked = actionClass.doProcess(
req,
resp);
 
RequestDispatcher rd = null;
 
if (itWorked) {
rd = req.getRequestDispatcher(action.getSuccessPage());
} else {
rd = req.getRequestDispatcher(action.getErrorPage());
}
try {
rd.forward(req, resp);
} catch (ServletException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
}

That way you only need the dispatch code in one place.

Similarly, I prefer to decouple the request and response objects, and instead pass in something like a Map of parameters and a Writer to write to.

If you do that you can then compile and test your Action classes without needing a Server, and without even needing to load the Servlet API classes.

4. I'm puzzled by the similarity in names between IAction and the Action class which appears later. In my experience things like this quickly act to confuse developers new to the project. Can you think of a more memorable name for one of them?

5. This one is really a personal preference. Can you explain a little why you chose to use XML for your configurations in a very light-weight framework like this? All the overhead of loading and using XML processing code when the same information could easily be represented as something like a simple properties file:

signin.class = com.ms.action.SignInAction
signin.success = welcome.jsp
signin.error = signIn.jsp
 
signup.class = com.ms.action.SignUpAction
signup.success = signUp.jsp
 
forgot.class = com.ms.action.ForgotPasswordAction
forgot.success = forgotPassword.jsp

You may have lots of reasons to prefer XML that are not apparent in this example, but I always recommend that such decisions are thought through carefully rather than just assumed.


That will do for now, I guess. Thanks for sharing your design with us.

[ankur rathi]

First of all, thank you very much for your valuable suggestions.
All suggestions/comments are very good and I will apply them surely.

Here are some reasons why I did it that way...

1. Is there any particular reason you use an I prefix on your Action interface? I personally find that names work best if they are 100% descriptive of their function, and 0% descriptive of their implementation.

The reason is, we have coding standard document for our project and there I found that it is easy to identify an interface if it has prefix I.

Could you please explain this a bit "I personally find that names work best if they are 100% descriptive of their function, and 0% descriptive of their implementation" with an example if possible.

2. defining your own "service" method is usually considered dangerous behaviour. Do you really want your Actions to respond to any old HEAD, PUT or OPTIONS request that appears at the server? Although it's clumsy, the best way is still usually to define one method (with whatever unclaimed name you like) and call it from redefined doGet and doPost methods.

One more disadvantage in this approach is I am not able to use *POST* behaviour, no security of password....

But I am not sure how HEAD, PUT or OPTION etc request will come to my server???

But I change it.

3. I also worry that you have coupled your actions a bit too close to the Servlet API. My preference is to decouple knowledge of servlet API methods from the business logic as it can make unit testing your code immensely easier. This is particularly apparent where you do the request dispatching in the Action.

You mean, I am passing request and response to my doProcess() method. But I need these objects in action class.

I can think of two options:

1] Get information from request in main controller class and then pass only those infomation (in form of map) to action class instead of request. But every action class needs different informationm, so I think it is not feasible...

2] If I pass a map that has request, response and other things to action class, then also the case will be same. I can't write Junit for my action class, because I can't hardcode that map myself....

What can be other options??

The code you have given is superb, it is saving me from duplication of code. But again we will have to pass request and response....

4. I'm puzzled by the similarity in names between IAction and the Action class which appears later. In my experience things like this quickly act to confuse developers new to the project. Can you think of a more memorable name for one of them?

I will change it and think some good name for xml storage object... or will change interface name...

5. This one is really a personal preference. Can you explain a little why you chose to use XML for your configurations in a very light-weight framework like this? All the overhead of loading and using XML processing code when the same information could easily be represented as something like a simple properties file:

Here you are prefectly right. I never thought of properties file. I thought this type of file can be a XML only. I will surely think about it.

Thank a lot again. I am very glad to have such mentors.


[ November 23, 2005: Message edited by: rathi ji ]

[dema rogatkin]

I like your MVC framework. I hope you will start a new project at sourceforge soon, and my company can be your first customer. I'd recommend to provide abstart classes for your interfaces to hide too much implementation details and work with simple call like display(error), display(email), and so on. Actually you have already got this recommendation.

[ankur rathi]

Originally posted by dema rogatkin:
I like your MVC framework. I hope you will start a new project at sourceforge soon, and my company can be your first customer. I'd recommend to provide abstart classes for your interfaces to hide too much implementation details and work with simple call like display(error), display(email), and so on. Actually you have already got this recommendation.

Thank you very much Dema.
Sorry I didn't get what you recommend. Please explain a bit more.
Thanks again.

[Frank Carver]

Originally posted by rathi ji:
The reason is, we have coding standard document for our project and there I found that it is easy to identify an interface if it has prefix I.

Could you please explain this a bit "I personally find that names work best if they are 100% descriptive of their function, and 0% descriptive of their implementation" with an example if possible.

I guess my question could be rephrased as: Why does the code that uses this interface, and the people reading this code, need to know that it is an interface? If you use it as the type of a variable, parameter or return value, it doesn't matter if it is an interface or a class, the usage and the meaning is just the same. The most important thing is that each name is as descriptive as possible of its purpose. If you really do need to know implementation details such as whether it is an interface or an abstract class (for example) you can always look it up. But in the great majority of code that simply makes no difference.

If I write something like:

Map parameters = new HashMap();

Is "Map" a class or an interface? I don't care, and I doubt that you do.

But I am not sure how HEAD, PUT or OPTION etc request will come to my server???

All the more reason to make sure that nothing unexpected happens if they do

You might be surprised how many systems issue a HEAD request before a GET or POST. HEAD is useful because it returns a set of HTTP headers without needing the page body, so it can be used to check things like modification dates without forcing the application to do potentially lengthy processing and data transfers. Look up any reasonable HTTP and/or WEBDAV reference to find out about the other possibilities.

You mean, I am passing request and response to my doProcess() method. But I need these objects in action class.

Do you really? Or do you just need somewhere to read parameters from, and somewhere to write output to? In my experience, 99% of web applications need no more than this.

I can think of two options:

1] Get information from request in main controller class and then pass only those infomation (in form of map) to action class instead of request. But every action class needs different informationm, so I think it is not feasible...

Although I have seen applications that laboriously extract values from the parameters and build a new Map to pass to business logic, this is a clumsy way to do it. The servlet API provides a simple way to get all the supplied parameters in one go using the getParameterMap method of a ServletRequest method.

2] If I pass a map that has request, response and other things to action class, then also the case will be same. I can't write Junit for my action class, because I can't hardcode that map myself....

I'm not sure I understand this. Putting some information into a Map to pass to some code is something I do every day in JUnit. Here's a short example showing how you could test something like your doProcess code without involving any of the Servlet API :

import junit.framework.TestCase;
import java.util.Map;
import java.util.HashMap;
import java.util.Writer;
import java.util.StringWriter;
 
import com.ms.action.IAction;
 
public class ParameterTest 
extends TestCase
{
  Map parameters;
  Writer output;
  IAction action;
 
  public void setUp()
  {
    parameters = new HashMap();
    output = new StringWriter();
  }
 
  public void testInvalidLogin()
  {
    parameters.put("username", "Frank");
    parameters.put("password", "huh");
    action = new SignInAction();
 
    assertFalse(action.doProcess(parameters, output));
    assertEquals("", output.toString());
  }
}

Obviously your code would need more tests than that, but I hope you can see how it might work.

[QB}
Thank a lot again. I am very glad to have such mentors.
[/QB]

You are welcome. That's what JavaRanch is here for!

[ankur rathi]

Thanks Frank.

I'm not sure I understand this. Putting some information into a Map to pass to some code is something I do every day in JUnit. Here's a short example showing how you could test something like your doProcess code without involving any of the Servlet API

No, I didn't mean that. I was thinking of a map that has request, response and some other objects in it. Someting like this:

public boolean doProcess(Map objects) {
     HttpServletRequest req = objects.get("req");
     String username = req.getParameter("username");
     // then do some processing...
}

Now in this case, Junit is not possible because it is as bad as passing request object directly.

But now it is clear to me, I will pass request parameters map instead of request object. But what is the need of writer object??? I think if I want to display any specific kind of error message then it is useful, I will write that into this writer.

Is this right and only use of this writer??

Now I am not overridding service() method. I am calling my own method from doGet() and doPost() both.

Regarding IAction interface and Action class (XML storage object), I have still not come up with any alternative name.

Thanks a lot again.

[ankur rathi]

Hi,

This is really very basic question but I am not clear. This is my signIn.jsp file:

 
<HTML>
<HEAD>
<TITLE>signIn.jsp</TITLE>
</HEAD>
<BODY>
<b>If you are a new user then click on <i>sign up</i> button <br>
If you are registered user then enter username and password and click on
<i>sign in</i> button <br>
If you are registered user but don't remember the password then click on
<i>forgot password</i> button
<br><br><br>
<form method="post" action="MainController">
username<INPUT type="text" name="username"/><br>
password <INPUT type="text" name="password"/><br>
<br>
<INPUT type="submit" value="Sign In">
<INPUT type="submit" value="Sign Up">
<INPUT type="submit" value="Forgot Password">
</form>
</b>
</BODY>
</HTML>
 

How can I send three different events from the same form?

In other words, There are three buttons at my page and I want different action on each click of each button. How could I do this??? or how can I know at server side that which button has been clicked by user???

I am sure there must be many ways of doing this, please tell me the simpler one and best one.

Thanks a lot.

[ankur rathi]

Here are 2nd version of my two files which has major changes.

 
package com.ms.controller;
 
import java.io.IOException;
import java.io.Writer;
import java.util.Map;
 
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
 
import com.ms.action.IAction;
import com.ms.actionloader.ActionModelLoader;
import com.ms.actionmodel.Action;
import com.ms.actionmodel.MailApplication;
 
public class MainController extends HttpServlet {
 
MailApplication mailApp = null;
 
public void init(ServletConfig arg0) throws ServletException {
super.init(arg0);
 
ActionModelLoader loader = new ActionModelLoader();
mailApp = loader.loadMailApplication("mail-application.xml");
 
}
 
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
super.doGet(req, resp);
process(req, resp);
}
 
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
super.doPost(req, resp);
process(req, resp);
}
 
protected void process(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
super.service(req, resp);
 
String type = req.getParameter("actiontype");
Action action = mailApp.getAction(type);
IAction actionClass = null;
try {
actionClass =
(IAction) Class.forName(action.getActionClass()).newInstance();
} catch (InstantiationException e) {
e.printStackTrace();
} catch (IllegalAccessException e) {
e.printStackTrace();
} catch (ClassNotFoundException e) {
e.printStackTrace();
}
Map reqParamaters = req.getParameterMap();
Writer writer = resp.getWriter();
 
boolean result = actionClass.doProcess(reqParamaters, writer);
RequestDispatcher rd = null;
 
if (result) {
rd = req.getRequestDispatcher(action.getSuccessPage());
} else {
rd = req.getRequestDispatcher(action.getErrorPage());
}
try {
rd.forward(req, resp);
} catch (ServletException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
 
}
}
 

package com.ms.action;
 
import java.io.Writer;
import java.util.Map;
 
public class SignInAction implements IAction {
 
public boolean doProcess(Map reqParameters, Writer writer) {
 
boolean result = false;
 
String username = (String) reqParameters.get("username");
String password = (String) reqParameters.get("password");
 
if (username != null && password != null) {
result = true;
} else {
result = false;
}
 
return result;
}
}
 

Now, I think I can write Junit for my action classes. No code duplication. Not overridding service() method...

But still getting this exception:

java.lang.IllegalStateException: Response already committed.

What can be the reason?

Thanks a lot.

[Frank Carver]

But now it is clear to me, I will pass request parameters map instead of request object. But what is the need of writer object??? I think if I want to display any specific kind of error message then it is useful, I will write that into this writer.

Is this right and only use of this writer??

You might not even need it in your application. Or you might need a different way of passing results back from your action handler to the framework. I'm afriad I don't know much about what you want to do with these actions, and what (if any) output values they might produce, so I can't help much beyond this point.

My own framework for this sort of thing (mojasef) does something a bit more complicated, which allows the action handler to write things back into the map of parameters which are then made available to the page that is eventually displayed. Other frameworks may work differently.

[ankur rathi]

Originally posted by Frank Carver:


You might not even need it in your application. Or you might need a different way of passing results back from your action handler to the framework. I'm afriad I don't know much about what you want to do with these actions, and what (if any) output values they might produce, so I can't help much beyond this point.

My own framework for this sort of thing (mojasef) does something a bit more complicated, which allows the action handler to write things back into the map of parameters which are then made available to the page that is eventually displayed. Other frameworks may work differently.

Yes, you are right. I am also thinking that my doProcess() method should return something more than boolean so I can use it somewhere. In that case, I even don't need to pass Writer object to my action classes. Because I can put any error message (or anything else) into list or map and then return that list or map....

Hope my point is clear.