Set of jQuery .click functions

Question

Restaurant Menu It works how I want it to, but the way I've written the jQuery looks like it could be simplified. How can I rewrite the jQuery so it does exactly the same thing but in much less code? I need only the first square of JavaScript rest is only an understanding of the code

$(document).ready(function() {
 //Variables
 var selectedStarter = {
 dish: "(None)",
 price: 0
 };
 var selectedMain = {
 dish: "(None)",
 price: 0
 };
 var selectedDessert = {
 dish: "(None)",
 price: 0
 };
 var starter = {
 firstDish: "Salad",
 firstDishPrice: 15,
 secondDish: "Soup",
 secondDishPrice: 7,
 thirdDish: "Fish rolls",
 thirdDishPrice: 12
 };
 var main = {
 firstDish: "Steak",
 firstDishPrice: 17,
 secondDish: "Salmon",
 secondDishPrice: 12,
 thirdDish: "Rissotto",
 thirdDishPrice: 9
 };
 var dessert = {
 firstDish: "Sorbet",
 firstDishPrice: 4,
 secondDish: "Fruit salad",
 secondDishPrice: 6,
 thirdDish: "Apple pie",
 thirdDishPrice: 5
 };
 function total() {
 return selectedStarter.price + selectedMain.price + selectedDessert.price;
 }
 function selectedStarterFnc(dish, price) {
 selectedStarter.price = price;
 selectedStarter.dish = dish;
 $("#total").html(total());
 return dish + "(" + price + ")";
 }
 function selectedMainFnc(dish, price) {
 selectedMain.price = price;
 selectedMain.dish = dish;
 $("#total").html(total());
 return dish + "(" + price + ")";
 }
 function selectedDessertFnc(dish, price) {
 selectedDessert.price = price;
 selectedDessert.dish = dish;
 $("#total").html(total());
 return dish + "(" + price + ")";
 }
 // Instantiating HTML Button Elements
 // Starter Elements
 document.getElementById("btStarter1").value =
 starter.firstDish + ": " + starter.firstDishPrice;
 document.getElementById("btStarter2").value =
 starter.secondDish + ": " + starter.secondDishPrice;
 document.getElementById("btStarter3").value =
 starter.thirdDish + ": " + starter.thirdDishPrice;
 // Main Elements
 document.getElementById("btMain1").value =
 main.firstDish + ": " + main.firstDishPrice;
 document.getElementById("btMain2").value =
 main.secondDish + ": " + main.secondDishPrice;
 document.getElementById("btMain3").value =
 main.thirdDish + ": " + main.thirdDishPrice;
 // Dessert Elements
 document.getElementById("btDessert1").value =
 dessert.firstDish + ": " + dessert.firstDishPrice;
 document.getElementById("btDessert2").value =
 dessert.secondDish + ": " + dessert.secondDishPrice;
 document.getElementById("btDessert3").value =
 dessert.thirdDish + ": " + dessert.thirdDishPrice;
 // Your Order: Elements
 document.getElementById("selectedStarter").innerHTML =
 selectedStarter.dish + " (" + selectedStarter.price + ")";
 document.getElementById("selectedMain").innerHTML =
 selectedMain.dish + " (" + selectedMain.price + ")";
 document.getElementById("selectedDessert").innerHTML =
 selectedDessert.dish + " (" + selectedDessert.price + ")";
 // Functions (JQuery)
 // Main menu onClicks handler
 $("#btMenu").click(function() {
 $("#liMainMenu").toggle("slow");
 });
 $("#btStarter").click(function() {
 $("#liStarter").toggle("slow", function() {
 if ($(this).css("display") == "none") {
 $("#btStarter").css("background-color", "#008080");
 } else {
 $("#btStarter").css("background-color", "red");
 }
 });
 });
 $("#btMain").click(function() {
 $("#liMain").toggle("slow", function() {
 if ($(this).css("display") == "none") {
 $("#btMain").css("background-color", "#008080");
 } else {
 $("#btMain").css("background-color", "red");
 }
 });
 });
 $("#btDessert").click(function() {
 $("#liDessert").toggle("slow", function() {
 if ($(this).css("display") == "none") {
 $("#btDessert").css("background-color", "#008080");
 } else {
 $("#btDessert").css("background-color", "red");
 }
 });
 });
 // Starter onClicks
 $("#btStarter1").click(function() {
 $("#liStarter").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedStarter").html(selectedStarterFnc(starter.firstDish, starter.firstDishPrice));
 });
 $("#btStarter2").click(function() {
 $("#liStarter").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedStarter").html(selectedStarterFnc(starter.secondDish, starter.secondDishPrice));
 });
 $("#btStarter3").click(function() {
 $("#liStarter").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedStarter").html(selectedStarterFnc(starter.thirdDish, starter.thirdDishPrice));
 });
 // Main onClicks
 $("#btMain1").click(function() {
 $("#liMain").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedMain").html(selectedMainFnc(main.firstDish, main.firstDishPrice));
 });
 $("#btMain2").click(function() {
 $("#liMain").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedMain").html(selectedMainFnc(main.secondDish, main.secondDishPrice));
 });
 $("#btMain3").click(function() {
 $("#liMain").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedMain").html(selectedMainFnc(main.thirdDish, main.thirdDishPrice));
 });
 // Dessert onClicks
 $("#btDessert1").click(function() {
 $("#liDessert").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedDessert").html(selectedDessertFnc(dessert.firstDish, dessert.firstDishPrice));
 });
 $("#btDessert2").click(function() {
 $("#liDessert").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedDessert").html(selectedDessertFnc(dessert.secondDish, dessert.secondDishPrice));
 });
 $("#btDessert3").click(function() {
 $("#liDessert").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 $("#selectedDessert").html(selectedDessertFnc(dessert.thirdDish, dessert.thirdDishPrice));
 });
});

.button {
 background: rgb(0, 230, 40);
 outline: solid 2px #353535;
 border: solid 2px white;
 color: white;
 padding: 10px 15px;
 text-decoration: none;
 width: 200px;
 margin-top: 50px;
}
.ul li {
 display: inline;
 margin-right: 8px;
}
.ul {
 display: none;
}
.table {
 margin-top: 50px;
 border: 10px solid blue;
 width: 43%;
}
.table th {
 padding-top: 20px;
 text-align: left;
 font-size: 24px;
 margin: 30px;
}
.table tr,
td {
 text-align: left;
 padding-left: 50px;
 padding-right: 20px;
 padding-top: 20px;
 padding-bottom: 20px
}

<!doctype html>
<html lang="en">
<head>
 <meta charset="utf-8">
 <title>Restaurant Menu With JQuery</title>
 <link rel="stylesheet" type="text/css" href="style.css">
 <script src="https://code.jquery.com/jquery-1.12.4.js"></script>
</head>
<body>
 <input type="button" class="button" id="btMenu" value="Menu" style="background-color: #353535">
 <ul id="liMainMenu" class="ul">
 <li>
 <input type="button" class="button" id="btStarter" value="Starter" style="background-color: #008080">
 </li>
 <li>
 <input type="button" class="button" id="btMain" value="Main" style="background-color: #008080">
 </li>
 <li>
 <input type="button" class="button" id="btDessert" value="Dessert" style="background-color: #008080">
 </li>
 </ul>
 <ul id="liStarter" class="ul">
 <li>
 <input type="button" class="button" id="btStarter1" value="">
 </li>
 <li>
 <input type="button" class="button" id="btStarter2" value="">
 </li>
 <li>
 <input type="button" class="button" id="btStarter3" value="">
 </li>
 </ul>
 <ul id="liMain" class="ul">
 <li>
 <input type="button" class="button" id="btMain1" value="">
 </li>
 <li>
 <input type="button" class="button" id="btMain2" value="">
 </li>
 <li>
 <input type="button" class="button" id="btMain3" value="">
 </li>
 </ul>
 <ul id="liDessert" class="ul">
 <li>
 <input type="button" class="button" id="btDessert1" value="">
 </li>
 <li>
 <input type="button" class="button" id="btDessert2" value="">
 </li>
 <li>
 <input type="button" class="button" id="btDessert3" value="">
 </li>
 </ul>
 <table class="table">
 <th>Your Order:</th>
 <tr>
 <td>First :</td>
 <td id="selectedStarter"></td>
 </tr>
 <tr>
 <td>Main :</td>
 <td id="selectedMain"></td>
 </tr>
 <tr>
 <td>Dessert :</td>
 <td id="selectedDessert"></td>
 </tr>
 <tr>
 <td>Total :</td>
 <td id="total"></td>
 </tr>
 </table>
 </body>
 <html>

enter image description here

• yuri: What does it do?
• yoni: Restaurant menu that update orders and change color button
• BCdotWEB: The site standard is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles.

Answer 1

Use better data structures.

When you have a problem you should try to find which data structure have more advantages for you, in this case i think that it should be an array.

Using an array.

The bigger advantage of using this approach is that is easier to remove or add new dishes to your menu, and you dont need to specificy the n-th of the dish, you can see that using 'first'/'second' would start to become difficult for a menu with 10 options or so.

Instead of:

var starter = {
 firstDish: "Salad",
 firstDishPrice: 15,
 secondDish: "Soup",
 secondDishPrice: 7,
 thirdDish: "Fish rolls",
 thirdDishPrice: 12 
 };

You can use it:

var starter = [{
 dish: "Salad",
 price: 15,
} { 
 dish: "Soup",
 price: 7,
}, {
 dish: "Fish rolls",
 price: 12
}];

And for free you now can make your events more abstract in a way to avoid repetition: There is one thing in HTML called data-*, its purpose is to hold some information that you need in order to identify the element; We can use it here:

<input type="button" class="starter-button" id="btStarter1" data-id="0" value="">

So as the array starts with 0, identification for the first dish is 0 as well. Now instead of creatring one event handler for each button we can abstract it to:

$(".starter-button").click(function() {
 // ...
 $(this).css("background-color", "red");
 $("#selectedDessert").html(selectedDessertFnc(starter[$(this).data("id")], starter[$(this).data("id")].price));
});

But this its just the start, you can use it better creating a even better data structure:

var dishes = [{
 "starter": [{
 name: "Salad",
 price: 15,
 }, { 
 name: "Soup",
 price: 7,
 }, {
 name: "Fish rolls",
 price: 12
 }],
 "desert": [...]
}]

Now using the same logic that before your button can be something like:

<input type="button" class="button" id="btStarter1" data-id="0" data-type="starter" data-target-id="selectedDessert" value="">

And your event:

$(".button").click(function() {
 // ... 
 var targetId = $(this).data("target-id");
 var dishType = $(this).data("type");
 var dishId = $(this).data("id");
 $(targetId).html(selectedDish(dishes[dishType][dishId].name, dishes[dishType][dishId].price);
});

The tl;dr version of my code review would be, Try to use the best data structures for your information, and think about how would the maintenance of the code, "how can i make it so its easier to add more features?".

After improving this bit your code should be pretty good, keep coding!

Answer 2

Like @Josenberg mentioned your data I believe would be better structured like:

var main = [{
 dish: "Steak",
 price: 17
},{
 dish: "Salmon",
 price: 12
},{
 dish: "Rissotto",
 price: 9
}];

Then to fill your elements you could use a single for loop.

for(var i=0;i<main.length;i++){
 document.getElementById("btMain"+(i+1).toString()).value =
 main[i].dish + ": " + main[i].price;
}

I would then add a class starter, main, and dessert to each of the respective elements. And then you could have a class on click function:

$('.starter').click(function(){
 $("#liStarter").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 var index = $(".starter").index(this);
 $("#selectedStarter").html(selectedStarterFnc(starter[index].dish, starter[index].price));
});

$(document).ready(function() {
 //Variables
 var selectedStarter = {
 dish: "(None)",
 price: 0
 };
 var selectedMain = {
 dish: "(None)",
 price: 0
 };
 var selectedDessert = {
 dish: "(None)",
 price: 0
 };
 var starter = [{
 dish: "Salad",
 price: 15
 },{
 dish: "Soup",
 price: 7
 },{
 dish: "Fish rolls",
 price: 12
 }];
 var main = [{
 dish: "Steak",
 price: 17
 },{
 dish: "Salmon",
 price: 12
 },{
 dish: "Rissotto",
 price: 9
 }];
 var dessert = [{
 dish: "Sorbet",
 price: 4
 },{
 dish: "Fruit salad",
 price: 6
 },{
 dish: "Apple pie",
 price: 5
 }];
 function total() {
 return selectedStarter.price + selectedMain.price + selectedDessert.price;
 }
 function selectedStarterFnc(dish, price) {
 selectedStarter.price = price;
 selectedStarter.dish = dish;
 $("#total").html(total());
 return dish + "(" + price + ")";
 }
 function selectedMainFnc(dish, price) {
 selectedMain.price = price;
 selectedMain.dish = dish;
 $("#total").html(total());
 return dish + "(" + price + ")";
 }
 function selectedDessertFnc(dish, price) {
 selectedDessert.price = price;
 selectedDessert.dish = dish;
 $("#total").html(total());
 return dish + "(" + price + ")";
 }
 // Instantiating HTML Button Elements
 // Starter Elements
 for(var i=0;i<starter.length;i++){
 document.getElementById("btStarter"+(i+1).toString()).value =
 starter[i].dish + ": " + starter[i].price;
 }
 // Main Elements
 for(var i=0;i<main.length;i++){
 document.getElementById("btMain"+(i+1).toString()).value =
 main[i].dish + ": " + main[i].price;
 }
 // Dessert Elements
 for(var i=0;i<dessert.length;i++){
 document.getElementById("btDessert"+(i+1).toString()).value =
 dessert[i].dish + ": " + dessert[i].price;
 }
 // Your Order: Elements
 document.getElementById("selectedStarter").innerHTML =
 selectedStarter.dish + " (" + selectedStarter.price + ")";
 document.getElementById("selectedMain").innerHTML =
 selectedMain.dish + " (" + selectedMain.price + ")";
 document.getElementById("selectedDessert").innerHTML =
 selectedDessert.dish + " (" + selectedDessert.price + ")";
 // Functions (JQuery)
 // Main menu onClicks handler
 $("#btMenu").click(function() {
 $("#liMainMenu").toggle("slow");
 });
 $("#btStarter").click(function() {
 $("#liStarter").toggle("slow", function() {
 if ($(this).css("display") == "none") {
 $("#btStarter").css("background-color", "#008080");
 } else {
 $("#btStarter").css("background-color", "red");
 }
 });
 });
 $("#btMain").click(function() {
 $("#liMain").toggle("slow", function() {
 if ($(this).css("display") == "none") {
 $("#btMain").css("background-color", "#008080");
 } else {
 $("#btMain").css("background-color", "red");
 }
 });
 });
 $("#btDessert").click(function() {
 $("#liDessert").toggle("slow", function() {
 if ($(this).css("display") == "none") {
 $("#btDessert").css("background-color", "#008080");
 } else {
 $("#btDessert").css("background-color", "red");
 }
 });
 });
 // Starter onClicks
 $('.starter').click(function(){
 $("#liStarter").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 var index = $(".starter").index(this);
 $("#selectedStarter").html(selectedStarterFnc(starter[index].dish, starter[index].price));
 });
 // Main onClicks
 $('.main').click(function(){
 $("#liMain").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 var index = $(".main").index(this);
 $("#selectedMain").html(selectedMainFnc(main[index].dish, main[index].price));
 });
 // Dessert onClicks
 $('.dessert').click(function(){
 $("#liDessert").children("li").children("input").css("background-color", "rgb(0, 230, 40)");
 $(this).css("background-color", "red");
 var index = $(".dessert").index(this);
 $("#selectedDessert").html(selectedDessertFnc(dessert[index].dish, dessert[index].price));
 });
});

.button {
 background: rgb(0, 230, 40);
 outline: solid 2px #353535;
 border: solid 2px white;
 color: white;
 padding: 10px 15px;
 text-decoration: none;
 width: 200px;
 margin-top: 50px;
}
.ul li {
 display: inline;
 margin-right: 8px;
}
.ul {
 display: none;
}
.table {
 margin-top: 50px;
 border: 10px solid blue;
 width: 43%;
}
.table th {
 padding-top: 20px;
 text-align: left;
 font-size: 24px;
 margin: 30px;
}
.table tr,
td {
 text-align: left;
 padding-left: 50px;
 padding-right: 20px;
 padding-top: 20px;
 padding-bottom: 20px
}

<!doctype html>
<html lang="en">
<head>
 <meta charset="utf-8">
 <title>Restaurant Menu With JQuery</title>
 <link rel="stylesheet" type="text/css" href="style.css">
 <script src="https://code.jquery.com/jquery-1.12.4.js"></script>
</head>
<body>
 <input type="button" class="button" id="btMenu" value="Menu" style="background-color: #353535">
 <ul id="liMainMenu" class="ul">
 <li>
 <input type="button" class="button" id="btStarter" value="Starter" style="background-color: #008080">
 </li>
 <li>
 <input type="button" class="button" id="btMain" value="Main" style="background-color: #008080">
 </li>
 <li>
 <input type="button" class="button" id="btDessert" value="Dessert" style="background-color: #008080">
 </li>
 </ul>
 <ul id="liStarter" class="ul">
 <li>
 <input type="button" class="button starter" id="btStarter1" value="">
 </li>
 <li>
 <input type="button" class="button starter" id="btStarter2" value="">
 </li>
 <li>
 <input type="button" class="button starter" id="btStarter3" value="">
 </li>
 </ul>
 <ul id="liMain" class="ul">
 <li>
 <input type="button" class="button main" id="btMain1" value="">
 </li>
 <li>
 <input type="button" class="button main" id="btMain2" value="">
 </li>
 <li>
 <input type="button" class="button main" id="btMain3" value="">
 </li>
 </ul>
 <ul id="liDessert" class="ul">
 <li>
 <input type="button" class="button dessert" id="btDessert1" value="">
 </li>
 <li>
 <input type="button" class="button dessert" id="btDessert2" value="">
 </li>
 <li>
 <input type="button" class="button dessert" id="btDessert3" value="">
 </li>
 </ul>
 <table class="table">
 <th>Your Order:</th>
 <tr>
 <td>First :</td>
 <td id="selectedStarter"></td>
 </tr>
 <tr>
 <td>Main :</td>
 <td id="selectedMain"></td>
 </tr>
 <tr>
 <td>Dessert :</td>
 <td id="selectedDessert"></td>
 </tr>
 <tr>
 <td>Total :</td>
 <td id="total"></td>
 </tr>
 </table>
 </body>
 <html>

• Josenberg: I had not thought of that but rendering the elements from the objects would be a good improvement as well.

Answer 3

Since you are using jQuery and have it tagged, I want to mention that you are missing an opportunity to use it. Every time you do document.getElementById(id), you could be using $('#id). This is more concise, allows you use jQuery's other methods like .html(), .val() and .data(), and makes it easier to change the selector without needing to fundamentally change the code.

For instance, instead of

document.getElementById("btStarter1").value = starter.firstDish + ": " + starter.firstDishPrice;

you could simply do

$('#btStarter1').val(starter.firstDish + ": " + starter.firstDishPrice);

If you then needed to access nodes by class instead of id, it would be straightforward to simply change the selector:

$('.starterButtons').val(starter.firstDish + ": " + starter.firstDishPrice);

• Matthew FitzGerald-Chamberlain: Can the downvoter explain why they don't agree?

Answer 4

As other answers have mentioned. use better data structures to organise your data

Check this out i have made a small fiddle.

https://jsfiddle.net/czpy2ze3/

$(document).ready(function(){
var dishes = {
 'starters': [
 {name: 'Salad', price: 15},
 {name: 'Fish rolls', price: 33},
 ],
 'main' : [
 {name: 'Steak', price: 30},
 ],
 'desserts': [
 {name: 'Apple Pie', price: 12},
 ]
};
var types = Object.keys(dishes);
var selected = {};
$("#xMenu").click(function(){
 $("#xContent").toggle();
});
var calculate = function() {
 $("#xBill").html("");
 var total = 0;
 $xBill = $("<div>", {});
 types.forEach(function(type, index) {
 var dishindex = selected[type];
 console.log(typeof dishindex);
 if(typeof dishindex == 'number'){
 var dish = dishes[type][dishindex];
 console.log(dish);
 total+= dish.price;
 $xBill.append($("<p>",{
 html: `${type} : ${dish.name}(${dish.price})`
 }));
 } else {
 }
 });
 $xBill.append($("<h3>", {
 html: `Total: ${total}`
 }))
 $("#xBill").append($xBill);
 console.log(total);
}
var render = function() {
 var $xTypes = $("<div>", {"class": "xType"});
 var $xDishes = $("<div>", {"class": "xDishes"});
 $("#xContent")
 .html("")
 .append($xTypes)
 .append($xDishes);
 types.forEach(function(type){
 var $x = $("<button>", { "class": "xTypes", "text": type});
 $xTypes.append($x);
 var $row = $("<div>", {});
 dishes[type].forEach(function(dish, index){
 var $dish = $("<button>", {
 "class": "dish",
 "text": dish.name,
 "click": function() {
 selected[type] = index;
 render();
 console.log(selected);
 }
 });
 if(selected[type] == index) {
 $dish.addClass("xchosen");
 }
 $row.append($dish);
 });
 $xDishes.append($row);
 $x.click(function(){
 $row.toggle();
 })
 });
 calculate();
};
render();
});

Instead of reducing code you should focus on not hardcoding the data part into your website.