I am new to Javascript and HTML. I have written code for a calculator, so review my code and points out pros and cons in it.
(function ($) {
var shiftflag = 0;
var fontreducer = 0;
$.fn.close = function () {
$("#dragging").remove();
};
$.fn.calc = function () {
this.remove();
$("body").append('<div id="dragging"><FORM NAME="calc" id="rocorners2"><TABLE BORDER=none id="calfields"><TR><TD><INPUT TYPE="text" id="inputarea" NAME="input" Size="16" autofocus><br><INPUT TYPE="button" NAME="one" class="calcinput" VALUE="1" ><INPUT TYPE="button" NAME="two" class="calcinput" VALUE="2" ><INPUT TYPE="button" NAME="three" class="calcinput" VALUE="3"><INPUT TYPE="button" NAME="plus" class="calcinput" VALUE="+" ><br><INPUT TYPE="button" NAME="four" class="calcinput" VALUE="4" ><INPUT TYPE="button" NAME="five" class="calcinput" VALUE="5"><INPUT TYPE="button" NAME="six" class="calcinput" VALUE="6" ><INPUT TYPE="button" NAME="minus" class="calcinput" VALUE="-"><br><INPUT TYPE="button" NAME="seven" class="calcinput" VALUE="7"><INPUT TYPE="button" NAME="eight" class="calcinput" VALUE="8"><INPUT TYPE="button" NAME="nine" class="calcinput" VALUE="9"><INPUT TYPE="button" NAME="times" class="calcinput" VALUE="*"><br><INPUT TYPE="button" NAME="clear" class="calcinput" VALUE="c"><INPUT TYPE="button" NAME="zero" class="calcinput" VALUE="0"><INPUT TYPE="button" NAME="DoIt" class="calcinput" VALUE="="><INPUT TYPE="button" NAME="div" class="calcinput" VALUE="/"><br></TD></TR></TABLE></FORM></div>');
$("#dragging").draggable();
var finding = (document).getElementById("inputarea");
finding.setAttribute('autocomplete', 'off'); {
currentfontsize = parseFloat($("#inputarea").css('font-size'));
$(".calcinput").click(function () {
var res = $(this).val();
var flag = 0;
if (res === 'c') {
flag = 1;
calc.input.value = "0";
}
if (res === '=') {
var c = eval(calc.input.value);
var text = document.getElementById("inputarea");
text.value = c;
} else {
if (calc.input.value == 0) {
calc.input.value = res;
} else
calc.input.value += res;
}
if (flag === 1) {
calc.input.value = '0';
}
currentfontsize = parseFloat($("#inputarea").css('font-size'));
funcc();
});
$("#calfields").keydown(function (e) {
var b = e.keyCode;
currentfontsize = parseFloat($("#inputarea").css('font-size'));
if (b === 13 || b === 187) {
e.preventDefault();
var c = eval(calc.input.value);
var text = document.getElementById("inputarea");
text.value = c;
} else {
document.getElementById("inputarea").style.fontSize = 34;
}
if (b === 16) {
shiftflag = 1;
}
if (b === 187 && shiftflag === 1) {
calc.input.value += '+';
e.preventDefault();
shiftflag = 0;
} else if (((b > 64 && b < 91) || (b === 192) || (b > 218 && b < 222))) {
e.preventDefault();
}
funcc();
});
//This function is to reduce the font size if there is any overflow in the text box area
function funcc() {
console.log(($('#inputarea')[0].scrollWidth > $('#inputarea').innerWidth()));
console.log(currentfontsize);
if (($('#inputarea')[0].scrollWidth > $('#inputarea').innerWidth())) {
if (fontreducer === 0 || currentfontsize === 34) {
currentfontsize = currentfontsize - 5;
fontreducer = 1;
} else if (fontreducer == 1 && currentfontsize > 13) {
currentfontsize = currentfontsize - 2;
} else if (currentfontsize < 13)
currentfontsize = currentfontsize + 0;
$("#inputarea").css({
'font-size': currentfontsize
});
}
return;
};
};
}
}(window.jQuery));
function manipulate_and_display(arr, operator) {
var loop;
var str = "";
if (arr[0] === undefined) {
calc.input.value = "";
} else {
for (loop = 0; loop < arr.length - 1; loop++) {
str = "" + str + "" + arr[loop] + "" + operator + "";
}
str = str + "" + arr[arr.length - 1];
calc.input.value = eval(str);
}
}
table {
cell-padding border: 0px;
cell-spacing border: 0px;
border-color: darkgrey;
color: 2px solid green;
}
.calcinput {
height: 50px;
font-size: 22px;
width: 50px;
color: black;
padding: 2px;
background-color: rgb(229, 236, 197);
border-radius: 0px;
}
#ruler {
visibility: hidden;
white-space: nowrap;
}
#inputarea {
text-align: left;
bottom: 20px;
height: 40px;
font-size: 34px;
width: 200px;
border-radius: 10px;
overflow: scroll;
}
#dragging {
border: 2px solid green;
width: 210px;
height: 267px;
}
#rocorners2 {
margin-top: 20px;
}
#calfields {
border: 1px;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.0.2/jquery.min.js"></script>
<html>
<head>
<script src="jquery.js"></script>
<link href="calc.css" rel="stylesheet">
<script src="http://code.jquery.com/ui/1.11.4/jquery-ui.js"></script>
<script src="calc.js"></script>
</head>
<body>
<h3>Calculator</h3>
<input type=button Name="display" value="Calculator" id="initialise">
<input type=button name="Close" value="close Calc" id="closing_the_calc">
<span id="ruler"></span>
</body>
<script>
$(document).ready(function () {
$("#initialise").click(function () {
var arr = new Array();
arr = [1, 2, 3, 4, 5];
$("#initialise").calc();
// If we pass the array with values it can do the specified operation by the operator you have passed and display it in the text box of calculator.
manipulate_and_display(arr, '*');
});
$("#closing_the_calc").click(function () {
$("#closing_the_calc").close();
});
});
</script>
</html>
1 Answer 1
HTML
script
elements are not allowed before the html
or after the body
element (neither are any other elements for the matter) . Depending on how the scripts are written, they belong all inside the head
element, or preferably at the end of (but inside) the body
element.
All script should be in external files. So either out the "document ready" code also into calc.js
, or put it (together with the function manipulate_and_display
) into a separate file.
JavaScript
manipulate_and_display
What exactly is the point of manipulate_and_display
? It's name doesn't expain what it does and it doesn't seem to have anything to do with the calculator directly.
The variable calc
in it isn't defined anywhere I can find, but It works, so currently I'm assuming it's an implicit variable created by giving the calculator form the name "calc". Don't do that. Using implicit variables created from element IDs and names an out-of-date way to access them. Always get element references using DOM methods such as getElementById()
(or store them when creating elements and reuse the references).
You unnecessarily use empty strings when concatinating. This:
str = "" + str + "" + arr[loop] + "" + operator + "";
could just as well be:
str = str + arr[loop] + operator;
Plugin
The whole point of (jQuery) plugins is, that you can reuse the elements and used them in different pages, possibly even multiple times in the same web page. However you by doing things such as hardcoding IDs and having variables such as shiftflag
in a higher scope so it can only exist once.
Speaking of shiftflag
: Don't use 0
/ 1
to represent booleans.
Defining a (separate) plugin with a generic name such as close
will overwrite (or be overwritten by) any other plugin that happens to have the same name.
In plugins you should allways assume that the jQuery object references multiple elements. You should either make sure it only contains one element, or better write your plugin so that it supports multiple elements by using jQuery.each()
.
Generally check out jQuery's tutorial for basic plugin development: https://learn.jquery.com/plugins/basic-plugin-creation/
(There is much more, but I don't have the time right now. Maybe I can come back later.)
Explore related questions
See similar questions with these tags.