Web application for recording blog comments

Question

I am working on some coursework for a simple Java EE web application that records blog comments. On the add comment page there are two fields: name and comments

I am using a RequestDispatcher to pass control to an acknowledgement.jsp if the fields are not empty, and an error.jsp if one (or both) are empty passing a relevant message to the acknowledgement page.

This is the logic I have:

 RequestDispatcher disp;
 if (!name.isEmpty() && !comment.isEmpty())
 {
 CommentBean commentBean = new CommentBean();
 commentBean.setComment(comment);
 commentBean.setName(name);
 req.setAttribute("comment", commentBean);
 disp = req.getRequestDispatcher("acknowledgement.jsp");
 disp.forward(req, resp);
 return;
 } 
 else if (!name.isEmpty() && comment.isEmpty())
 {
 disp = req.getRequestDispatcher("error.jsp");
 statusMessage = "Comment field empty";
 req.setAttribute("status", statusMessage);
 disp.forward(req, resp);
 }
 else if (name.isEmpty() && !comment.isEmpty())
 {
 disp = req.getRequestDispatcher("error.jsp");
 statusMessage = "Name field empty";
 req.setAttribute("status", statusMessage);
 disp.forward(req, resp);
 }
 else
 {
 disp = req.getRequestDispatcher("error.jsp");
 statusMessage = "Name and comment fields empty";
 req.setAttribute("status", statusMessage);
 disp.forward(req, resp);
 }

The code does what I want but I can't help thinking this could be written a lot better although I am not sure how to go about it.

• Svetlin Zarev: It;s a small remark, so I won;t post it as an answer: Use proper indentation -> brackets should start on the same line

Answer 1

That's easy: You're repeating quite a few things, so don't do it.

disp.forward(req, resp);

This is the easiest, copied in each of the 4 branches, but needn't be. Drop the needless return and move it after the branches.

But in general, deduplication is hard and it's much better to learn to avoid duplications. Just imagine Ctrl-C and Ctrl-V are broken and you must type using one finger only. :D You could write a method accepting statusMessage and handling the problem. But it's simple enough so you can do it inline.

The basic idea is "compute the statusMessage, leave it empty in case there's no problem and handle it":

if (comment.isEmpty()) {
 statusMessage = name.isEmpty()
 ? "Name and comment fields empty"
 : "Comment field empty";
} else {
 statusMessage = name.isEmpty()
 ? "Name field empty"
 : "";
}
if (statusMessage.isEmpty()) {
 CommentBean commentBean = new CommentBean();
 commentBean.setComment(comment);
 commentBean.setName(name);
 req.setAttribute("comment", commentBean);
 disp = req.getRequestDispatcher("acknowledgement.jsp");
} else {
 disp = req.getRequestDispatcher("error.jsp");
}
disp.forward(req, resp);