1
\$\begingroup\$

I have this jQuery form with 4 text inputs.
The first one only accepts digits, the second one formats value into 'de-DE' price format accepting 2 decimals, the second does the same but accepts 5 decimals after decimal separator, fourth one automatically adds periods for date input.

    $(window).ready(function () {
           		function validateNumber(event) {
					var $txtBox = $(this);
					var key = window.event ? event.keyCode : event.which;
                      //activates the Backspace button and right/left arrows
					if (event.keyCode == 8 || event.keyCode == 9 || event.keyCode == 37 || event.keyCode == 39) {
						return true;
					} // prevent if not number/dot
						else if ( key < 44 || key > 57 && event.keyCode != 44) {
						event.preventDefault();
					}else {
					var len = $txtBox.val().length;
					var index = $txtBox.val().indexOf(',');
	
					if (index > 0 && event.keyCode == 44) {
					return false;
					}
					if (index > 0) {
					var charAfterdot = (len + 1) - index;
					if (charAfterdot > 6) {
						return false;
					}
				}
			}
						return $txtBox;
				    };
            $('[id^=int]').keypress(validateNumber);
			$('[id^=float1]').keypress(validateNumber);
			$('[id^=float1]').on("blur", numberFormat);
			
			  function numberFormat(event) {
			     var number = parseFloat(this.value);
					console.log(this.value);
					console.log("before: " + number);
					console.log("after: " + number.toLocaleString('de-DE',{maximumFractionDigits: 5, style:'currency', currency:'EUR'}));
						this.value = isNaN(number) ? '0': number.toLocaleString('de-DE',{maximumFractionDigits:5, style:'currency', currency:'EUR'});
						var digits = maximumFractionDigits;
				};
			$('[id^=float2]').keypress(validateNumber);
			$('[id^=float2]').on("blur", numberFormat);
		   var dt = dateFormatter('#date');
		    	$('[id^=date]').keypress(validateNumber);
        });
<!DOCTYPE html>
<html>
<head> 
	<meta charset="utf-8"/>
		<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>    
		<script src="dateFormatter.js"></script>
		<script>
        </script>

 
</head>
<body style="position:relative;">
		<form style="display:block;margin:0 auto;border:1px solid #aaa;border-radius:10px;width:215px;padding:10px;margin-top:100px;background-color:#F0F0F0;box-shadow:2px 2px 2px 2px rgba(0,0,0,0.2);">
			<table>
				<tr>
					<td>Number:</td>
					<td><input id="int" type="input" value="0" size="5" maxlength="13" style=width:100px;margin-left:5px;,/></td>
				</tr>
				<tr>
					<td>FloatComma1:</td>
					<td><input id="float1" decimalPlaces="2" type="text" maxlength="13" style=width:100px;margin-left:5px;/></td>
				</tr>
				<tr>
					<td>FloatComma2:</td>
					<td><input id="float2" decimalPlaces="5" type="text" maxlength="13" style=width:100px;margin-left:5px;/></td>
				</tr>
				<tr>
					<td>Date:</td>
					<td><input id="date" type="text" value="" maxlength="10"  style=width:100px;margin-left:5px; /></td>
				</tr>
			</table>
		</form>
	</body>

As you can see, the FloatComma1 variable is allowing more than 2 decimals because my numberFormat function has this.value = number.toLocaleString('de-DE',{maximumFractionDigits:5});, so it allows 5.

Does anyone know a way how I could just replace the 5 with, let´s say a variable decimals, which I can give separate values to?
For example, I could call the variable in FloatComma1 decimals = 3 , and in FloatComma2 decimals = 5?

I do not know how to do it. Can someone help?

\$\endgroup\$
7
  • 3
    \$\begingroup\$ I'm voting to close this question as off-topic because it is a feature request instead of a request for Code Review. \$\endgroup\$
    – Mast ♦
    Jun 16, 2015 at 14:13
  • \$\begingroup\$ Mast, then this is very confusing and I have to say everyone has different opinions here and it´s just a matter of who is reviewing the questions. Last week I had posted a question on SO needing help about an cursor-following animation and pasted my code there. It got downvoted and everyone got so annoyed because they said it does not belong to SO and the admins moved it to codereview. So...i´m not really getting the point in here. The code with the animation was also not complete but still moved here and it also had a question. Explain? \$\endgroup\$
    – Daria M
    Jun 16, 2015 at 14:23
  • 2
    \$\begingroup\$ @DariaM Stack Overflow users are not the same as Code Review users. Could you provide a link to the question that was suggested for CR? \$\endgroup\$
    – Simon Forsberg
    Jun 16, 2015 at 14:24
  • 1
    \$\begingroup\$ At the moment, I consider this question a grey area for Code Review, I do feel however that it is on the right side of that grey area. You have working code, you know a refactoring you would like to do but you just need some assistance doing it. I personally think this can be left open. \$\endgroup\$
    – Simon Forsberg
    Jun 16, 2015 at 14:26
  • 1
    \$\begingroup\$ @DariaM In that question, you are basically asking "How can I make this code better?" (in this case smaller). Here some of us feel that the question is "How can I add a feature to my code so that I can use different values for this?" (which is a feature request and is off-topic) and some of us feel that the question is "How can I refactor my code to make this value, which is currently a constant, more flexible?" (which I think is on-topic) -- that's why this question is in a grey area. This question only has three close votes (of the required 5) so far though. \$\endgroup\$
    – Simon Forsberg
    Jun 16, 2015 at 14:35

1 Answer 1

Reset to default
6
\$\begingroup\$

I'm solely going to post on your coding structure rather than the method of procedure for the task your code does.

In validateNumber(), your indentation starts out good and then goes downhill from there.

}else {
var len = $txtBox.val().length;
var index = $txtBox.val().indexOf(',');
if (index > 0 && event.keyCode == 44) {
return false;
}
if (index > 0) {
var charAfterdot = (len + 1) - index;
if (charAfterdot > 6) {
        return false;
}

with whitespace added would transform into:

} else {
    var len = $txtBox.val().length;
    var index = $txtBox.val().indexOf(',');
    if (index > 0 && event.keyCode == 44) {
        return false;
    }
    if (index > 0) {
        var charAfterdot = (len + 1) - index;
        if (charAfterdot > 6) {
            return false;
        }

Your naming conventions could be improved also:
For example, charAfterdot into charAfterDot.

Instead of blindly comparing random event.keyCodes with numbers, declare the numbers before hand as constants. 

const BACKSPACE = 8;
const TAB = 9;
const LEFT_ARROW = 37;
const RIGHT_ARROW = 39;

However, your version of JavaScript may not support constants, so use var in that case, and then just:

if (event.keyCode == BACKSPACE || event.keyCode == TAB || event.keyCode == LEFT_ARROW || event.keyCode == RIGHT_ARROW)

Declaring constants is a much clearer way of programming, than comparing to random numbers.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks, I think you did a realy constructive remark. I made most of the changes that you proposed, but you lost me at declaring constants. Because I tried it like so and the letters were typing as well. Maybe I did it wrong. I deleted this line var key = window.event ? event.keyCode : event.which; and added the constants. I replaced my if statement as you sugested and replaced the key with event.keyCode. \$\endgroup\$
    – Daria M
    Jun 16, 2015 at 15:06
  • 1
    \$\begingroup\$ Older versions of JavaScript doesn't support the const keyword AFAIK. They might have to be declared as var \$\endgroup\$
    – Simon Forsberg
    Jun 16, 2015 at 18:26

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