Skip to main content
+ magic numbers, date validation
Source Link
Sumurai8
  • 2.7k
  • 11
  • 19

###Date validation Your script happily accepts dates such as "31-03", even though there is no March 31rd. I am unsure how you would deal with this, as you allow multiple placeholders of the same type, and one would normally input the day before the month.

###Date validation Your script happily accepts dates such as "31-03", even though there is no March 31rd. I am unsure how you would deal with this, as you allow multiple placeholders of the same type, and one would normally input the day before the month.

+ magic numbers
Source Link
Sumurai8
  • 2.7k
  • 11
  • 19

Note: There is quite some code, and I have not looked at all of it. Using this review you can probably find quite some loose ends yourself.

##Styling and readability Single quote and double quote strings behave identical in javascript. I recommend to choose one of the two, and stick with it. Either escape the single or double quote when you need to use it instead of switching to the other type string.

You are declaring variables using the comma syntax. I recommend to instead use var for every variable instead, because the comma syntax is prone to errors. If you forget a comma, or have a semicolon instead, you drop some of your variables into the global scope. This can happen quite easily when modifying the variables in a particular part of your program.

You sometimes put else on a new line, and you sometimes put it on the same line as the previous }. I would recommend choosing one style, and sticking to it.

You are using some magic numbers, mainly "48" and "57". I would recommend replacing them with constants, and initializing those constants somewhere at the top. For example with "0".charCodeAt(0);.

##Logic errors / bugs You are strictly comparing parameters with the string literal "undefined". You likely meant to use typeof parameter === "undefined" or parameter === undefined.

Your script does not catch the backspace or delete keys. Using these keys will result in the input to be broken until the page reloads. You probably should replace deleted characters with their placeholder counterparts.

Your script allows for invalid dates, such as "39" for "DD" if you first enter "19", then go back and press "3". You probably have to reset the next character to "D" when you type the first.

##Other possible improvements ###Optional parameters You can use the ternary operator for conditional assignment to condense some of the code, such as in the following case:

format = (typeof format === "undefined") ? looseconfig.format : parseDateTime(format);

parseDateTime(..) does not have a default case, and the split regex does not account for multiple delimiters. This will cause multiple delimiters to be put in the validation string. This does not seem to play well with the dateTimeRules(..) function, where you "just take the next char" if you hit a delimiter.

###NaN You are checking if a character in the validation string is a delimited by doing !Number(thisChar). Your validation string can currently not contain a 0, but this can change. I recommend changing this to Number.isNaN(Number(thisChar)) to better show what you want to accomplish, and to avoid a bug when your validation string for some reason can contain a 0. Alternatively, because your validation string seems to be always a /, why don't you just compare that?

Later, you are using !thisChar.isNaN, but I am not aware of such a property on a String. It thus will always be true.

###Double character in regex You have the / character twice in your regex in parseDateTime(..).

dateTime.split(/[\/\/:.\s]/g);

###Duplicated code In dateTimeRules(..) you do explode = val.split("");. In the if-statement, you do this again. I suppose you got halfway moving duplicated code out of the if-statement, but forgot to remove it from the if-statement.

Note: There is quite some code, and I have not looked at all of it. Using this review you can probably find quite some loose ends yourself.

##Styling and readability Single quote and double quote strings behave identical in javascript. I recommend to choose one of the two, and stick with it. Either escape the single or double quote when you need to use it instead of switching to the other type string.

You are declaring variables using the comma syntax. I recommend to instead use var for every variable instead, because the comma syntax is prone to errors. If you forget a comma, or have a semicolon instead, you drop some of your variables into the global scope. This can happen quite easily when modifying the variables in a particular part of your program.

You sometimes put else on a new line, and you sometimes put it on the same line as the previous }. I would recommend choosing one style, and sticking to it.

##Logic errors / bugs You are strictly comparing parameters with the string literal "undefined". You likely meant to use typeof parameter === "undefined" or parameter === undefined.

Your script does not catch the backspace or delete keys. Using these keys will result in the input to be broken until the page reloads. You probably should replace deleted characters with their placeholder counterparts.

Your script allows for invalid dates, such as "39" for "DD" if you first enter "19", then go back and press "3". You probably have to reset the next character to "D" when you type the first.

##Other possible improvements ###Optional parameters You can use the ternary operator for conditional assignment to condense some of the code, such as in the following case:

format = (typeof format === "undefined") ? looseconfig.format : parseDateTime(format);

parseDateTime(..) does not have a default case, and the split regex does not account for multiple delimiters. This will cause multiple delimiters to be put in the validation string. This does not seem to play well with the dateTimeRules(..) function, where you "just take the next char" if you hit a delimiter.

###NaN You are checking if a character in the validation string is a delimited by doing !Number(thisChar). Your validation string can currently not contain a 0, but this can change. I recommend changing this to Number.isNaN(Number(thisChar)) to better show what you want to accomplish, and to avoid a bug when your validation string for some reason can contain a 0. Alternatively, because your validation string seems to be always a /, why don't you just compare that?

Later, you are using !thisChar.isNaN, but I am not aware of such a property on a String. It thus will always be true.

###Double character in regex You have the / character twice in your regex in parseDateTime(..).

dateTime.split(/[\/\/:.\s]/g);

###Duplicated code In dateTimeRules(..) you do explode = val.split("");. In the if-statement, you do this again. I suppose you got halfway moving duplicated code out of the if-statement, but forgot to remove it from the if-statement.

Note: There is quite some code, and I have not looked at all of it. Using this review you can probably find quite some loose ends yourself.

##Styling and readability Single quote and double quote strings behave identical in javascript. I recommend to choose one of the two, and stick with it. Either escape the single or double quote when you need to use it instead of switching to the other type string.

You are declaring variables using the comma syntax. I recommend to instead use var for every variable instead, because the comma syntax is prone to errors. If you forget a comma, or have a semicolon instead, you drop some of your variables into the global scope. This can happen quite easily when modifying the variables in a particular part of your program.

You sometimes put else on a new line, and you sometimes put it on the same line as the previous }. I would recommend choosing one style, and sticking to it.

You are using some magic numbers, mainly "48" and "57". I would recommend replacing them with constants, and initializing those constants somewhere at the top. For example with "0".charCodeAt(0);.

##Logic errors / bugs You are strictly comparing parameters with the string literal "undefined". You likely meant to use typeof parameter === "undefined" or parameter === undefined.

Your script does not catch the backspace or delete keys. Using these keys will result in the input to be broken until the page reloads. You probably should replace deleted characters with their placeholder counterparts.

Your script allows for invalid dates, such as "39" for "DD" if you first enter "19", then go back and press "3". You probably have to reset the next character to "D" when you type the first.

##Other possible improvements ###Optional parameters You can use the ternary operator for conditional assignment to condense some of the code, such as in the following case:

format = (typeof format === "undefined") ? looseconfig.format : parseDateTime(format);

parseDateTime(..) does not have a default case, and the split regex does not account for multiple delimiters. This will cause multiple delimiters to be put in the validation string. This does not seem to play well with the dateTimeRules(..) function, where you "just take the next char" if you hit a delimiter.

###NaN You are checking if a character in the validation string is a delimited by doing !Number(thisChar). Your validation string can currently not contain a 0, but this can change. I recommend changing this to Number.isNaN(Number(thisChar)) to better show what you want to accomplish, and to avoid a bug when your validation string for some reason can contain a 0. Alternatively, because your validation string seems to be always a /, why don't you just compare that?

Later, you are using !thisChar.isNaN, but I am not aware of such a property on a String. It thus will always be true.

###Double character in regex You have the / character twice in your regex in parseDateTime(..).

dateTime.split(/[\/\/:.\s]/g);

###Duplicated code In dateTimeRules(..) you do explode = val.split("");. In the if-statement, you do this again. I suppose you got halfway moving duplicated code out of the if-statement, but forgot to remove it from the if-statement.

typo
Source Link
Sumurai8
  • 2.7k
  • 11
  • 19

Note: There is quite some code, and I have not looked at all of it. Using this review you can probably find quite some loose ends yourself.

##Styling and readability Single quote and double quote strings behave identical in javascript. I recommend to choose one of the two, and stick with it. Either escape the single or double quote when you need to use it instead of switching to the other type string.

You are declaring variables using the comma syntax. I recommend to instead use var for every variable instead, because the comma syntax is prone to errors. If you forget a comma, or have a semicolon instead, you drop some of your variables into the global scope. This can happen quite easily when modifying the variables in a particular part of your program.

You sometimes put else on a new line, and you sometimes put it on the same line as the previous }. I would recommend choosing one style, and sticking to it.

##Logic errors / bugs You are strictly comparing parameters with the string literal "undefined". You likely meant to use typeof parameter === "undefined" or parameter === undefined.

Your script does not catch the backspace or delete keys. Using these keys will result in the input to be broken until the page reloads. You probably should replace deleted characters with their placeholder counterparts.

Your script allows for invalid dates, such as "39" for "DD" if you first enter "19", then go back and press "3". You probably have to reset the next character to "D" when you type the first.

##Other possible improvements ###Optional parameters You can use the ternary operator for conditional assignment to condense some of the code, such as in the following case:

format = (typeof format === "undefined") ? looseconfig.format : parseDateTime(format);

parseDateTime(..) does not have a default case, and the split regex does not account for multiple delimiters. This will cause multiple delimiters to be put in the validation string. This does not seem to play well with the dateTimeRules(..) function, where you "just take the next char" if you hit a delimiter.

###NaN You are checking if a character in the validation string is a delimited by doing !Number(thisChar). Your validation string can currently not contain a 0, but this can change. I recommend changing this to Number.isNaN(Number(thisChar)) to better show what you want to accomplish, and to avoid a bug when your validation string for some reason can contain a 0. Alternatively, because your validation string seems to be always a /, why don't you just compare that?

Later, you are using !thisChar.isNaN, but I am not aware of such a property on a String. It thus will always will always be true.

###Double character in regex You have the / character twice in your regex in parseDateTime(..).

dateTime.split(/[\/\/:.\s]/g);

###Duplicated code In dateTimeRules(..) you do explode = val.split("");. In the if-statement, you do this again. I suppose you got halfway moving duplicated code out of the if-statement, but forgot to remove it from the if-statement.

Note: There is quite some code, and I have not looked at all of it. Using this review you can probably find quite some loose ends yourself.

##Styling and readability Single quote and double quote strings behave identical in javascript. I recommend to choose one of the two, and stick with it. Either escape the single or double quote when you need to use it instead of switching to the other type string.

You are declaring variables using the comma syntax. I recommend to instead use var for every variable instead, because the comma syntax is prone to errors. If you forget a comma, or have a semicolon instead, you drop some of your variables into the global scope. This can happen quite easily when modifying the variables in a particular part of your program.

You sometimes put else on a new line, and you sometimes put it on the same line as the previous }. I would recommend choosing one style, and sticking to it.

##Logic errors / bugs You are strictly comparing parameters with the string literal "undefined". You likely meant to use typeof parameter === "undefined" or parameter === undefined.

Your script does not catch the backspace or delete keys. Using these keys will result in the input to be broken until the page reloads. You probably should replace deleted characters with their placeholder counterparts.

Your script allows for invalid dates, such as "39" for "DD" if you first enter "19", then go back and press "3". You probably have to reset the next character to "D" when you type the first.

##Other possible improvements ###Optional parameters You can use the ternary operator for conditional assignment to condense some of the code, such as in the following case:

format = (typeof format === "undefined") ? looseconfig.format : parseDateTime(format);

parseDateTime(..) does not have a default case, and the split regex does not account for multiple delimiters. This will cause multiple delimiters to be put in the validation string. This does not seem to play well with the dateTimeRules(..) function, where you "just take the next char" if you hit a delimiter.

###NaN You are checking if a character in the validation string is a delimited by doing !Number(thisChar). Your validation string can currently not contain a 0, but this can change. I recommend changing this to Number.isNaN(Number(thisChar)) to better show what you want to accomplish, and to avoid a bug when your validation string for some reason can contain a 0. Alternatively, because your validation string seems to be always a /, why don't you just compare that?

Later, you are using !thisChar.isNaN, but I am not aware of such a property on a String. It thus will always will always be true.

###Double character in regex You have the / character twice in your regex in parseDateTime(..).

dateTime.split(/[\/\/:.\s]/g);

###Duplicated code In dateTimeRules(..) you do explode = val.split("");. In the if-statement, you do this again. I suppose you got halfway moving duplicated code out of the if-statement, but forgot to remove it from the if-statement.

Note: There is quite some code, and I have not looked at all of it. Using this review you can probably find quite some loose ends yourself.

##Styling and readability Single quote and double quote strings behave identical in javascript. I recommend to choose one of the two, and stick with it. Either escape the single or double quote when you need to use it instead of switching to the other type string.

You are declaring variables using the comma syntax. I recommend to instead use var for every variable instead, because the comma syntax is prone to errors. If you forget a comma, or have a semicolon instead, you drop some of your variables into the global scope. This can happen quite easily when modifying the variables in a particular part of your program.

You sometimes put else on a new line, and you sometimes put it on the same line as the previous }. I would recommend choosing one style, and sticking to it.

##Logic errors / bugs You are strictly comparing parameters with the string literal "undefined". You likely meant to use typeof parameter === "undefined" or parameter === undefined.

Your script does not catch the backspace or delete keys. Using these keys will result in the input to be broken until the page reloads. You probably should replace deleted characters with their placeholder counterparts.

Your script allows for invalid dates, such as "39" for "DD" if you first enter "19", then go back and press "3". You probably have to reset the next character to "D" when you type the first.

##Other possible improvements ###Optional parameters You can use the ternary operator for conditional assignment to condense some of the code, such as in the following case:

format = (typeof format === "undefined") ? looseconfig.format : parseDateTime(format);

parseDateTime(..) does not have a default case, and the split regex does not account for multiple delimiters. This will cause multiple delimiters to be put in the validation string. This does not seem to play well with the dateTimeRules(..) function, where you "just take the next char" if you hit a delimiter.

###NaN You are checking if a character in the validation string is a delimited by doing !Number(thisChar). Your validation string can currently not contain a 0, but this can change. I recommend changing this to Number.isNaN(Number(thisChar)) to better show what you want to accomplish, and to avoid a bug when your validation string for some reason can contain a 0. Alternatively, because your validation string seems to be always a /, why don't you just compare that?

Later, you are using !thisChar.isNaN, but I am not aware of such a property on a String. It thus will always be true.

###Double character in regex You have the / character twice in your regex in parseDateTime(..).

dateTime.split(/[\/\/:.\s]/g);

###Duplicated code In dateTimeRules(..) you do explode = val.split("");. In the if-statement, you do this again. I suppose you got halfway moving duplicated code out of the if-statement, but forgot to remove it from the if-statement.

Source Link
Sumurai8
  • 2.7k
  • 11
  • 19
Loading