Shopping Cart - cannot add more than one item

[Shahid Kahn]

I have following servlet to add items into cart -

import java.io.*;
import java.io.IOException;
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 javax.servlet.RequestDispatcher;

/**
 * Servlet implementation class CartServlet
 */
@WebServlet("/cart")
public class CartServlet extends HttpServlet {
	private static final long serialVersionUID = 1L;
       
	/**
	 * @see HttpServlet#doGet(HttpServletRequest request, HttpServletResponse response)
	 */
	protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        String action = request.getParameter("act");
		ShoppingCartItem item = null;
		ShoppingCart cart = null;
		
		String productId = request.getParameter("productId");
		String productName = request.getParameter("productName");
		float price = Float.parseFloat(request.getParameter("mrp"));
		int qty = Integer.parseInt(request.getParameter("qty"));
		
		HttpSession session = request.getSession();

		session.setMaxInactiveInterval(1800); // 30 minutes 			

		if (session.getAttribute("cart") == null){
			cart = new ShoppingCart();
			session.setAttribute("cart", cart);
		}
		if(item == null){
			item = new ShoppingCartItem();
			item.setProductId(productId);
			item.setProductName(productName);		
			item.setPrice(price);
			item.setQuantity(qty);
		}
		
        if (action == null)
            action = "8";

        	/****** ADD Product ******/
        if (action.equals("1")){			
			synchronized(session){
				cart.add(item);
				cart.setNumberOfItems();
				cart.setCartTotal();
				session.setAttribute("cart", cart);
				session.setAttribute("cartInfo1", "Items: "+ cart.getNumberOfItems());
				session.setAttribute("cartInfo2", "Tk. " + cart.getCartTotal());		
			}		
		}
        
    	/***** UPADATE Quantity ********/
    	if(action.equals("2")){			
			synchronized(session){
				cart.updateQuantity(item);
			}		
    	}

    		/********* REMOVE Product *********/
    	if(action.equals("3")){
			synchronized(session){
				cart.removeItem(productId);
			}		    		
    	}
    	
		/********* REMOVE ALL Products *********/
    	if(action.equals("4")){
    		synchronized(session){
				cart.removeAll();
			}		    		
		}

        RequestDispatcher view = request.getRequestDispatcher("displayProducts.jsp?cid=(String)session.getAttrcurrentCat(currentCat)");
        view.forward(request, response);
	}

	/**
	 * @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse response)
	 */
	protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
		doGet(request, response);
	}

}

One item gets added to the cart successfully. When adding the second item it gives a Null Pointer Exception at line 53

cart.add(item);

Can anyone please help me out with the problem.

[Tim Cooke]

Your NullPointerException is because 'cart' is null on this line:

cart.add(item);

To simplify things, let's just look at the code that deals with the 'cart' variable:

ShoppingCart cart = null;
if (session.getAttribute("cart") == null) {
    cart = new ShoppingCart();  
    session.setAttribute("cart", cart);  
}
cart.add(item);

Can you see under what scenario 'cart' will be null?

[Tim Cooke]

And, welcome to the Ranch!

[Shahid Kahn]

Thanks Tim.

Do I have to add cart = session.getAttribute("cart"); before cart.add(item);

       if (action.equals("1")){              
            synchronized(session){  
                cart = session.getAttribute("cart");
                cart.add(item);  
                .........
                .........
       }

[Tim Cooke]

Yes, you got it.

But... I wouldn't do it inside that if statement because you're going to run into the same NullPointerException when your action equals "2". I suggest doing it close to where you are doing the null check on the session attribute. Like so:

ShoppingCart cart = null;
if (session.getAttribute("cart") == null) {
    session.setAttribute("cart", new ShoppingCart());
}
cart = session.getAttribute("cart");
// Now cart is ready to use from this point on.

[Shahid Kahn]

Thanks again. This time for quick replies.

I will try it. I will update the post after trying.

[J. Kevin Robbins]

Incidentally, this is poor design. It violates the http specifications. Decide which method you are going to use and let the other one throw a 405 Method Not Allowed error. Or if you want to use both methods in the same servlet (opinions vary here) then they should do distinctly different operations in keeping with the spec. Use GET to retrieve data (idempotent) and POST to submit data that will change the state of the application such as adding records to a database or items to a shopping cart (non idempotent).

    /** 
     * @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse response) 
     */  
    protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {  
        doGet(request, response);  
    } 

And welcome to the Ranch!

[Shahid Kahn]

Thanks Kevin for your suggestion.

It does not give me any exception any more after the change. But it increments the quantity of the product added in the cart on browser refresh. How solve this issue?

[Tim Cooke]

That's because you're using a GET request for adding things to the cart. You have a URL such as:
<your-server>/cart?act=1&productId=47&productName=PlushMoose&mrp=100&qty=3

Every time you submit that URL, you will put something in the cart.

A solution is to submit a POST request instead. Which brings us round nicely to what Kevin said yesterday, that you should use GET to retrieve data and POST to submit data, where submitting data is what you're doing in this case.

(Side note: Having the item price in the request is a bad idea. I could modify the request to put any price I want in there and you don't want people to be able to do that)

[Shahid Kahn]

This is my JSP code to add item in the cart

			<c:forEach var="product" items="${requestScope.productList}">
				<div class="prodDiv">
					<div class="prodinfo">
						<div class="prodName"><p>${product.productName}</p></div>
						<div class="prodMrp"><p><b>Tk. ${product.mrp}</b></p></div>
					</div>
					<div class="submitinfo">
						<form action="/cart" method="post">							
							<input type="hidden" name="productId" value="${product.productId}" >
							<input type="hidden" name="productName" value="${product.productName}" >
							<input type="hidden" name="mrp" value="${product.mrp}" >
							<input type="hidden" name="act" value="1" >
							
							<div style="float: left; width: 38px; text-align: left; margin-top: 3px;"><i>Qty: </i></div><div id="qtydiv" style="float: left; "><input type="number" name="qty" min="1" value="1" id="qtyfield"></div>
							<div id="buttondiv"><input type="submit" name="submit" value="Add to Cart" id="cartButton"></div>
						</form>
					</div>
				</div>			
			</c:forEach>

[Tim Cooke]

... and ... what's your question?

[Shahid Kahn]

Ohh...
I am not using GET method. I am using POST method with some hidden to pass to Servlet. Why does it increment the quantity of the added product in the cart?

[Tim Cooke]

After you have submitted your form to add an item into the cart, what do you have in the browser address bar?

[Shahid Kahn]

The address bar has something like bellow as you mentioned.

<your-server>/cart?act=1&productId=47&productName=PlushMoose&mrp=100&qty=3

Should I use JavaBean to pass parameter values to servlet instead of hidden field? Should it solve the issue?

[Tim Cooke]

I do not have your code running so cannot verify, but this looks suspicious:

RequestDispatcher view = request.getRequestDispatcher("displayProducts.jsp?cid=(String)session.getAttrcurrentCat(currentCat)");  
view.forward(request, response);

Did you mean:

RequestDispatcher view = request.getRequestDispatcher("displayProducts.jsp?cid=" + (String)session.getAttribute("currentCat"));  
view.forward(request, response);

?

[Shahid Kahn]

Yes, you are right.
displayProducts.jsp displays all the products from selected category of products retrieved from database. here "cid" means category id. products are retrieved from database every time displayProduct.jsp is loaded.

is there anything wrong with this?

[Tim Cooke]

Does that change resolve your refresh form submission problem?

[Shahid Kahn]

i gotta test it. i will update the post after testing. thanks.
it was a great help and support from you Tim.