Simple Java web app code

I wanted to work on HttpSessions and JSP.

This is the view I have:

    

    
    
    
    
        
         
        ">Add to basket
        
    
    
    Currently your cart contains these items dude:
    

    
        
         
        
        
    

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 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 doSessionStuff(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) {
HttpSession session = httpServletRequest.getSession();
Map cartMap = n

Interfaces vs implementations

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

private static HashMap products = new HashMap();

Do like this:

private static Map products = new HashMap();

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 doSessionStuff(HttpServletRequest httpServletRequest) {
    HttpSession session = httpServletRequest.getSession();
    if (session.getAttribute("cart") == null) {
        session.setAttribute("cart", new HashMap());
    }
    return (Map) 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 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");
}

StackExchange Code Review Q#57524, answer score: 5