How can I make this code shorter?
function tblcheckboxes(){
var a= 0;
var b= 0;
var c= 0;
var d= 0;
var e= 0;
var f= 0;
var g= 0;
if ($('#tcbx1').is(":checked")) {
a = parseFloat($("#tcbx1").val(), 10);
}
if ($('#tcbx2').is(":checked")) {
b = parseFloat($("#tcbx2").val(), 10);
}
if ($('#tcbx3').is(":checked")) {
c = parseFloat($("#tcbx3").val(), 10);
}
if ($('#tcbx4').is(":checked")) {
d = parseFloat($("#tcbx4").val(), 10);
}
if ($('#tcbx5').is(":checked")) {
e = parseFloat($("#tcbx5").val(), 10);
}
if ($('#tcbx6').is(":checked")) {
f = parseFloat($("#tcbx6").val(), 10);
}
if ($('#tcbx7').is(":checked")) {
g = parseFloat($("#tcbx7").val(), 10);
}
var total = a + b + c + d + e + f + g;
$('.txt7').val(total.toFixed(2));
}
$(document).ready(function(){
$('#tcbx1').click(function(){
tblcheckboxes();
grandtotal();
});
$('#tcbx2').click(function(){
tblcheckboxes();
grandtotal();
});
$('#tcbx3').click(function(){
tblcheckboxes();
grandtotal();
});
$('#tcbx4').click(function(){
tblcheckboxes();
grandtotal();
});
$('#tcbx5').click(function(){
tblcheckboxes();
grandtotal();
});
$('#tcbx6').click(function(){
tblcheckboxes();
grandtotal();
});
$('#tcbx7').click(function(){
tblcheckboxes();
grandtotal();
});
});
-
\$\begingroup\$ Welcome to Code Review! I'm sure there is a way to simplify it. I hope you get some good reviews! \$\endgroup\$Phrancis– Phrancis2015年01月08日 01:47:28 +00:00Commented Jan 8, 2015 at 1:47
4 Answers 4
You could try to do the whole thing on the fly, like this:
var total = 0;
$("input.tcbx").change(function(){
if($(this).is(":checked")) {
total += parseFloat($(this).val(), 10);
} else {
total -= parseFloat($(this).val(), 10);
}
$('.txt7').val(total.toFixed(2));
});
Just add a class to all your checkboxes, and then either add the value to your total or reduce it as they get selected. If you have preselected values you might want to enter the preselected total already, otherwise you could use an each() to first check all the values.
$(document).ready(function(){
var total = 0;
$("input.tcbx").change(function(){
if($(this).is(":checked")) {
total += parseFloat($(this).val(), 10);
} else {
total -= parseFloat($(this).val(), 10);
}
$('#result').val(total.toFixed(2));
});
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<input type="checkbox" value="1" id="check-1" class="tcbx" />
<label for="check-1">1</label>
<input type="checkbox" value="2" id="check-2" class="tcbx" />
<label for="check-2">2</label>
<input type="checkbox" value="3" id="check-3" class="tcbx" />
<label for="check-3">3</label>
<input type="text" id="result">0</div>
For your document.ready function, I recommend using a loop, rather than going through each individual element:
$(document).ready(function() {
for(var i = 1; i <= 7; i++) {
$("#tcbx" + i).click(function() {
tblcheckboxes();
grandtotal();
});
}
});
And the same goes to your tblcheckboxes function, along with a better way to initialize, store, and add your variables:
function tblcheckboxes() {
var a = b = c = d = e = f = g = 0;
var vars = [a, b, c, d, e, f, g]; // store in an array for easier access
var total = 0; // initialize total so it can be added while looping through the elements
for(var i = 1; i <= 7; i++) {
if($("#tcbx" + i).is(":checked")) {
vars[i - 1] = parseFloat($("#tcbx" + i).val(), 10);
}
total += vars[i - 1];
}
$(".txt7").val(total.toFixed(2));
}
You can simplify and shorten code by using a container element for your checkboxes and just use one click handler on that (using jquery event delegation). Within the handler you can calculate the total for all checked checkboxes values. The snippet is a demo for it.
In it, the handler:
- gets the collection of all checked checkboxes within the container
- converts them to an array
- uses the
Array.reducemethod to calculate the total - displays the calculated total within
div#total
The advantage for this approach is also that you can add checkboxes dynamically, which are included immediately in the calculations. The second handler in the snippet demonstrates that.
$('#pricing').on( 'click', '[type=checkbox]', function (e) {
// all checked checkboxes in #pricing
var totamount = $('#totalamount').hide();
var total = $('#pricing > [type=checkbox]:checked')
.toArray() //<= convert to array
.reduce( function (a, b) { //<= calculate sum
return a + (+b.value); //<= + operator converts to Number
},0 );
totamount.html( total.toFixed(2) ).fadeIn();
});
$('#addcbx').on('click', function (e) {
var amount = $('#cbvalue').val();
if (!amount || amount < 0.001) {
return true;
}
var nwcbx = $('<input type="checkbox" value="' + amount + '" \/> '+ amount+ '<br>')
.insertBefore($('#total'));
});
body {
margin: 2em;
font: 0.8em/1em normal verdana, arial;
}
#total {
width: 150px;
border-top: 1px solid #ddd;
padding-top: 5px;
}
#totalamount {
font-weight: bold;
color: green;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="pricing">
<input type="checkbox" value=100.00 /> 100.00<br>
<input type="checkbox" value=231.00 /> 231.00<br>
<input type="checkbox" value=521.99 /> 521.99<br>
<input type="checkbox" value=22.25 /> 22.25<br>
<input type="checkbox" value=36.55 /> 36.55<br>
<input type="checkbox" value=96.32 /> 96.32<br>
<input type="checkbox" value=5.50 /> 5.50<br>
<div id="total">total: <span id="totalamount">0</div>
</div>
<p>
<input type="number" id="cbvalue" placeholder="amount" step="0.01" min="0.00" value="0.00"/>
<button id="addcbx">add checkbox for amount</button>
</p>
I would not use IDs, but a identifier for all your checkboxes, a class, obviously.
And I would store each values in an array instead of many vars.
$(function() {
// The collections of elements :
var $tcbxCollection = $('.tcbx');
// get the values of this collection :
var tcbxValues = getCheckboxesValues($tcbxCollection);
$('body' /* or the parent container */ ).on('click', function() {
tcbxValues = getCheckboxesValues($tcbxCollection);
console.log(tcbxValues);
});
// The function to put the important values.
function getCheckboxesValues($collection) {
var checkboxValues = [];
$collection.each(function(i, element) {
checkboxValues[i] = $(element).is(':checked') ?
parseFloat($(element).attr('value'), 10) : 0;
});
return checkboxValues;
}
});
<input class="tcbx" type="checkbox" value="10.66666" />
<input class="tcbx" type="checkbox" value="28.45368789" />
<input class="tcbx" type="checkbox" value="32.5" />
<input class="tcbx" type="checkbox" value="4.5456" />
<input class="tcbx" type="checkbox" value="54.786" />
<input class="tcbx" type="checkbox" value="65" />
<input class="tcbx" type="checkbox" value="7.56464" />
<input class="tcbx" type="checkbox" value="8.7789" />
<p>Look on the console...</p>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
-
1\$\begingroup\$ I do have to add that this will mean that whenever a user clicks, the function will run. It's best practise to only do it when something actually happens, so maybe your
$('body').clickshould be$tcbxCollection.clickor$tcbxCollection.change, and not aparent.clickto save your resources. Only calculate when something happens, thats why I used thechange()event, it is tailor-made for this. \$\endgroup\$somethinghere– somethinghere2015年01月09日 17:53:26 +00:00Commented Jan 9, 2015 at 17:53