5
\$\begingroup\$

I need help in making this shopping cart adhere to the W3C rules. This is already 100% functioning. I just need help in making this code as clean and as short as possible. I want it in pure JavaScript and not in jQuery. I heard that you can use DOM instead of innerHTML. I've used innerHTML since I had a hard time using it in DOM. Also, if there are other possible mistakes, please help me change my code.

I hope you can review whatever mistakes that I made. You can reply by putting your edited code in snippet. I just started studying JavaScript and I hope you can help me with that.

    var products = [];
    var cart = [];

        function addProduct() {
            var productID = document.getElementById("productID").value;
            var product_desc = document.getElementById("product_desc").value;
            var qty = document.getElementById("quantity").value;
            var price = document.getElementById("price").value;

            var newProduct = {
                product_id: null,
                product_desc: null,
                product_qty: 0,
                product_price: 0.00,
            };
            newProduct.product_id = productID;
            newProduct.product_desc = product_desc;
            newProduct.product_qty = qty;
            newProduct.product_price = price;


            products.push(newProduct);


            var html = "<table border='1|1' >";
            html += "<td>Product ID</td>";
            html += "<td>Product Description</td>";
            html += "<td>Quantity</td>";
            html += "<td>Price</td>";
            html += "<td>Action</td>";
            for (var i = 0; i < products.length; i++) {
                html += "<tr>";
                html += "<td>" + products[i].product_id + "</td>";
                html += "<td>" + products[i].product_desc + "</td>";
                html += "<td>" + products[i].product_qty + "</td>";
                html += "<td>" + products[i].product_price + "</td>";
                html += "<td><button type='submit' onClick='deleteProduct(\"" + products[i].product_id + "\", this);'/>Delete Item</button> &nbsp <button type='submit' onClick='addCart(\"" + products[i].product_id + "\", this);'/>Add to Cart</button></td>";
                html += "</tr>";
            }
            html += "</table>";
            document.getElementById("demo").innerHTML = html;

            document.getElementById("resetbtn").click()
        }
        function deleteProduct(product_id, e) {
            e.parentNode.parentNode.parentNode.removeChild(e.parentNode.parentNode);
            for (var i = 0; i < products.length; i++) {
                if (products[i].product_id == product_id) {
                    // DO NOT CHANGE THE 1 HERE
                    products.splice(i, 1);
                }
            }
        }

        function addCart(product_id) {

            for (var i = 0; i < products.length; i++) {
                if (products[i].product_id == product_id) {
                    var cartItem = null;
                    for (var k = 0; k < cart.length; k++) {
                        if (cart[k].product.product_id == products[i].product_id) {
                            cartItem = cart[k];
                            cart[k].product_qty++;
                            break;
                        }
                    }
                    if (cartItem == null) {
                        
                        var cartItem = {
                            product: products[i],
                            product_qty: products[i].product_qty 
                        };
                        cart.push(cartItem);
                    }
                }
            }

            renderCartTable();

        }

        function renderCartTable() {
        var html = '';
        var ele = document.getElementById("demo2");
        ele.innerHTML = ''; 

        html += "<table id='tblCart' border='1|1'>";
        html += "<tr><td>Product ID</td>";
        html += "<td>Product Description</td>";
        html += "<td>Quantity</td>";
        html += "<td>Price</td>";
        html += "<td>Total</td>";
        html += "<td>Action</td></tr>";
        var GrandTotal = 0;
        for (var i = 0; i < cart.length; i++) {
            html += "<tr>";
            html += "<td>" + cart[i].product.product_id + "</td>";
            html += "<td>" + cart[i].product.product_desc + "</td>";
            html += "<td>" + cart[i].product_qty + "</td>";
            html += "<td>" + cart[i].product.product_price + "</td>";
            html += "<td>" + parseFloat(cart[i].product.product_price) * parseInt(cart[i].product_qty) + "</td>";
            html += "<td><button type='submit' onClick='subtractQuantity(\"" + cart[i].product.product_id + "\", this);'/>Subtract Quantity</button> &nbsp<button type='submit' onClick='addQuantity(\"" + cart[i].product.product_id + "\", this);'/>Add Quantity</button> &nbsp<button type='submit' onClick='removeItem(\"" + cart[i].product.product_id + "\", this);'/>Remove Item</button></td>";
            html += "</tr>";

           GrandTotal += parseFloat(cart[i].product.product_price) * parseInt(cart[i].product_qty);            

        }
        document.getElementById('demo3').innerHTML = GrandTotal;
        html += "</table>";
        ele.innerHTML = html;
    }


        function subtractQuantity(product_id)
        {
            
            for (var i = 0; i < cart.length; i++) {
                if (cart[i].product.product_id == product_id) {
                    cart[i].product_qty--;
                }

                if (cart[i].product_qty == 0) {
                    cart.splice(i,1);
                }
        
            }
            renderCartTable();
        }
        function addQuantity(product_id)
        {
            
            for (var i = 0; i < cart.length; i++) {
                if (cart[i].product.product_id == product_id) {
                    cart[i].product_qty++;
                }  
            }
            renderCartTable();
        }
        function removeItem(product_id)
        {
            
            for (var i = 0; i < cart.length; i++) {
                if (cart[i].product.product_id == product_id) {
                    cart.splice(i,1);
                }

            }
            renderCartTable();
        }
<!DOCTYPE html>
<html>
<head>
    <title>Shopping Cart Pure Javascript</title>
</head>
<body>
<form name="order" id="order">
    <table>
        <tr>
            <td>
                <label for="productID">Product ID:</label>
            </td>
            <td>
                <input id="productID" name="product" type="text" size="28" required/>
            </td>
        </tr>
        <tr>
            <td>
                <label for="product">Product Desc:</label>
            </td>
            <td>
                <input id="product_desc" name="product" type="text" size="28" required/>
            </td>
        </tr>
        <tr>
            <td>
                <label for="quantity">Quantity:</label>
            </td>
            <td>
                <input id="quantity" name="quantity" width="196px" required/>
            </td>
        </tr>
        <tr>
            <td>
                <label for="price">Price:</label>
            </td>
            <td>
                <input id="price" name="price" size="28" required/>
            </td>
        </tr>
    </table>
    <input type="reset" name="reset" id="resetbtn" class="resetbtn" value="Reset" />
    <input type="button" id="btnAddProduct" onclick="addProduct();" value="Add New Product" >
</form>
<br>
<p id="demo"></p> <br/>
<h2> Shopping Cart </h2>
<p id="demo2"></p>
<h2>Grand Total:</h2>
<p id="demo3"></p>
</body>
</html>

\$\endgroup\$
  • \$\begingroup\$ I am able to add any product I want and for a price of $0? \$\endgroup\$ – Ashton Morgan Jul 10 '17 at 17:06
  • \$\begingroup\$ @Ashton Morgan Please help me fix my code. Thanks \$\endgroup\$ – Thomas Jul 11 '17 at 1:00
  • \$\begingroup\$ This looks to be the same question as the one I answered here codereview.stackexchange.com/questions/168857/… feel free to take a look at the code I added there. \$\endgroup\$ – Chris Wright Jul 13 '17 at 15:41
  • \$\begingroup\$ Possible duplicate of Adding and Deleting in ES6 Javascript Alone \$\endgroup\$ – Icepickle Jul 22 '17 at 8:56
  • \$\begingroup\$ @ChrisWright and Icepickle: These questions are not duplicates. The other question contains revised code, and in particular, it uses ES6 features. \$\endgroup\$ – 200_success Jul 31 '17 at 16:42
3
\$\begingroup\$

Go here: https://validator.w3.org/ to check your markup. You can input it directly and it will show all your errors, just go down the list until you deal with them all (luckily, you only have a few).

As for making the code cleaner, I suggest you use ES6 where things like JavaScript template literals can help. Additionally, you can run cleaner looking loops on arrays with:

for (let product of products) {
  if (product.product_id === product_id) {

  }
}

By the way, note my use of 'strict equality' with the "===" in the 'if' statement? I suggest you go with that unless you have a solid reason not to.

Not that it applies here but there can be issues with onclick; alternatively, you can use event listeners.

You don't specify what kind of browser support you're concerned with but you may want to check out template tags (you can use a script tag for older browsers). This provides a good example:

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template

You say your code is "already 100% functional" but you aren't checking any inputs fields. This means anyone can enter any kind of data they want, not good, especially for a shopping cart (try regex).

I would enclose the code in an IIFE:

let myCart = (function() {
   // your code goes here
})()

Doing so ensures that your code will not be in the global space, thus preventing unforeseen and unwanted collisions.

Edit: I just tried out your example and I suggest that you put the "Add to Cart" button outside of the "Add New Product" display area. In other words, only add to the cart once, in bulk, as opposed to doing it on individual items. As of now, you can end up with a condition where I just removed an item from the upper display, yet...

http://imgur.com/a/jQbSq

\$\endgroup\$
  • \$\begingroup\$ Sir can you make changes on my code on a snippet using ES6. I would gladly appreciate it. Thank you \$\endgroup\$ – Thomas Jul 10 '17 at 16:07
  • 1
    \$\begingroup\$ I could... but then, how would you learn ;) \$\endgroup\$ – Steve Carroll Jul 10 '17 at 16:27
  • \$\begingroup\$ Sir please convert it to ES6. Just this time. Please. Thank you \$\endgroup\$ – Thomas Jul 10 '17 at 16:33
  • 2
    \$\begingroup\$ @Thomas You can't force anyone to do your work for you. \$\endgroup\$ – Simon Forsberg Jul 22 '17 at 9:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.