Simple Java web app code

Question 1

I wanted to work on HttpSessions and JSP.

This is the view I have:

<!DOCTYPE html>
<html lang="en">
<head>
 <meta charset="utf-8">
</head>
<body>
<div>
 <c:out value="All available products:"/>
 <br/>
 <br/>
 <c:forEach items="${applicationScope.allProducts}" var="product">
 <c:out value="${product}"/>
 &nbsp;
 <a href="${pageContext.servletContext.contextPath}/shop?addProductbyName=<c:out value="${product}"/>">Add to basket</a>
 <br/>
 </c:forEach>
 <br/>
 Currently your cart contains these items dude:
 <br/>
 <c:forEach items="${sessionScope.cart}" var="product">
 <c:out value="${product.key}"/>
 &nbsp;
 <c:out value="${product.value}"/>
 <br/>
 </c:forEach>
</div>
</body>
</html>

And the Servlet:

package com.tugay;
import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
/**
 * User: koraytugay
 * Date: 20/07/14
 * Time: 18:07
 */
@WebServlet(urlPatterns = "/shop", name = "shoppingServlet")
public class ShopServlet extends HttpServlet {
 @Override
 public void init() throws ServletException {
 if (getServletContext().getAttribute("allProducts") == null) {
 getServletContext().setAttribute("allProducts", Database.getAllProducts());
 }
 }
 @SuppressWarnings("unchecked")
 @Override
 protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse)
 throws ServletException, IOException {
 Map<String, Integer> cart = doSessionStuff(httpServletRequest, httpServletResponse);
 String addProductbyName = httpServletRequest.getParameter("addProductbyName");
 if (addProductbyName != null) {
 if (cart.get(addProductbyName) == null) {
 cart.put(addProductbyName, 0);
 }
 cart.put(addProductbyName, cart.get(addProductbyName) + 1);
 }
 httpServletResponse.sendRedirect(getServletContext().getContextPath()+"/shopping.jsp");
 }
 @SuppressWarnings("unchecked")
 private Map<String, Integer> doSessionStuff(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) {
 HttpSession session = httpServletRequest.getSession();
 Map<String, Integer> cartMap = null;
 if (session.getAttribute("cart") == null) {
 cartMap = new HashMap<String, Integer>();
 session.setAttribute("cart", cartMap);
 }
 cartMap = (Map<String, Integer>) session.getAttribute("cart");
 return cartMap;
 }
}

And the "Mock" Db:

package com.tugay;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
/**
 * User: koraytugay
 * Date: 20/07/14
 * Time: 17:58
 */
public class Database {
 private static HashMap<Integer,String> products = new HashMap<Integer, String>();
 static {
 products.put(1,"Table");
 products.put(2,"Sofa");
 products.put(3,"Chair");
 products.put(4,"Window");
 }
 public static List<String> getAllProducts() {
 return new ArrayList<String>(products.values());
 }
}

This is what my page looks like:

enter image description here

If anyone wants to review this, they are welcome.

Question 2

Interfaces vs implementations

Always use interfaces in declarations, not implementations. Instead of:

 private static HashMap<Integer,String> products = new HashMap<Integer, String>();

Do like this:

private static Map<Integer,String> products = new HashMap<Integer, String>();

doSessionStuff

The httpServletResponse parameter is unused, so drop it.

No need to initialize cartMap to null, as you will assign to it anyway. Actually you don't need that local variable at all, you could simplify the method without it:

@SuppressWarnings("unchecked")
private Map<String, Integer> doSessionStuff(HttpServletRequest httpServletRequest) {
 HttpSession session = httpServletRequest.getSession();
 if (session.getAttribute("cart") == null) {
 session.setAttribute("cart", new HashMap<String, Integer>());
 }
 return (Map<String, Integer>) session.getAttribute("cart");
}

doGet

You only use the cart variable if addProductbyName != null, so it would be better to move it inside the if block.

It's a bit awkward to do cart.put(addProductbyName, 0); and then again cart.put(addProductbyName, cart.get(addProductbyName) + 1);. And you are doing cart.get(addProductbyName) multiple times.

The @SuppressWarnings("unchecked") is also unnecessary for this method, as there are no unchecked casts.

The method would be cleaner like this:

@Override
protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse)
 throws ServletException, IOException {
 String addProductbyName = httpServletRequest.getParameter("addProductbyName");
 if (addProductbyName != null) {
 Map<String, Integer> cart = doSessionStuff(httpServletRequest);
 Integer count = cart.get(addProductbyName);
 if (count == null) {
 cart.put(addProductbyName, 1);
 } else {
 cart.put(addProductbyName, count + 1);
 }
 }
 httpServletResponse.sendRedirect(getServletContext().getContextPath()+"/shopping.jsp");
}

Question 3

In the JSP, there are a couple areas where you are inconsistant in your style. For example, you are outputting strings via

<c:out value="All available products:"/>

vs

 Currently your cart contains these items dude:

Also, you are mixing/matching c:out and EL string interpolation in your url construction here. You can simplify it:

<a href="${pageContext.servletContext.contextPath}/shop?addProductbyName=${product}"/>">Add to basket</a>

Question 4

So this is not required: <c:out value="${product}" ?

Question 5

Nope, just like you are able to add the contextPath in the same way.

Question 6

Is there any better way to make this link only with <c:url and maybe c:param ?

Question 7

It is quite important to stress the difference between using c:out or not. Always use c:out for dynamic data, @KorayTugay. See stackoverflow.com/questions/291031/jsp-cout-tag

janos janos 113k15 gold badges154 silver badges396 bronze badges · Accepted Answer · 2014-07-20 16:39:31Z

Interfaces vs implementations

Always use interfaces in declarations, not implementations. Instead of:

 private static HashMap<Integer,String> products = new HashMap<Integer, String>();

Do like this:

private static Map<Integer,String> products = new HashMap<Integer, String>();

doSessionStuff

The httpServletResponse parameter is unused, so drop it.

No need to initialize cartMap to null, as you will assign to it anyway. Actually you don't need that local variable at all, you could simplify the method without it:

@SuppressWarnings("unchecked")
private Map<String, Integer> doSessionStuff(HttpServletRequest httpServletRequest) {
 HttpSession session = httpServletRequest.getSession();
 if (session.getAttribute("cart") == null) {
 session.setAttribute("cart", new HashMap<String, Integer>());
 }
 return (Map<String, Integer>) session.getAttribute("cart");
}

doGet

You only use the cart variable if addProductbyName != null, so it would be better to move it inside the if block.

It's a bit awkward to do cart.put(addProductbyName, 0); and then again cart.put(addProductbyName, cart.get(addProductbyName) + 1);. And you are doing cart.get(addProductbyName) multiple times.

The @SuppressWarnings("unchecked") is also unnecessary for this method, as there are no unchecked casts.

The method would be cleaner like this:

@Override
protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse)
 throws ServletException, IOException {
 String addProductbyName = httpServletRequest.getParameter("addProductbyName");
 if (addProductbyName != null) {
 Map<String, Integer> cart = doSessionStuff(httpServletRequest);
 Integer count = cart.get(addProductbyName);
 if (count == null) {
 cart.put(addProductbyName, 1);
 } else {
 cart.put(addProductbyName, count + 1);
 }
 }
 httpServletResponse.sendRedirect(getServletContext().getContextPath()+"/shopping.jsp");
}

Stack Exchange Network

Simple Java web app code

2 Answers 2

Interfaces vs implementations

doSessionStuff

doGet

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Hot Network Questions

Simple Java web app code

2 Answers 2

Interfaces vs implementations

doSessionStuff

doGet

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Related

Hot Network Questions