JavaScript date validation

I have a date control which has a dropdown for each date, month and year.

The issue is, I can select 31st Nov. 2015 as well, and when I create date object, I get 1st Dec. 2015.

Example



var _date = 31; // value fetched from dropdown
var _month = 10;
var _year = 2015;

var date = new Date(_year, _month, _date);
document.write(date)



I want to validate this condition, and my current approach is to match date.getDate() with _date and show an error message if they are different.

Is there a better way to implement this?

Code



var _monthList = [
{id:"1",name:"January"} ,
{id:"2",name:"February"} ,
{id:"3",name:"March"} ,
{id:"4",name:"April"} ,
{id:"5",name:"May"} ,
{id:"6",name:"June"},
{id:"7",name:"July"} ,
{id:"8",name:"August"} ,
{id:"9",name:"September"} ,
{id:"10",name:"October"} ,
{id:"11",name:"November"} ,
{id:"12",name:"December"}];

function createYears(){
var _year = [];
for (var i = 2000; i";
$.each(dataset, function(key, value){
var id = value.id? value.id:key;
var text = value.name;
var val = value.value?value.value:value.id;
optionStr += ""+ text + ""
});
return optionStr;
}

function registerEvents(){
$("select").on("change", function(e){
validateDate();
});
}

function validateDate(){
var _date = $("#cbDate option:selected").val();
var _month = $("#cbMonth option:selected").val();
var _year = $("#cbYear option:selected").val();
if(_year && _month && _date){
var dateObj = new Date(_year, _month-1, _date);
if(_date != dateObj.getDate()){
$("#lblError").text("Please Select a valid date");
}
else{
$("#lblError").text("");
}
}
}

(function(){
createYears();
createMonths();
createDates();
registerEvents();
})()
`select{
background:#fff;
color:blue;
width:100px;
padding:5px;
border-radius:4px;
border: 1px solid

The logic you're using for validating a date looks fine to me.

The code is not bad, but here are a few suggestions for improvement:

IIFE

All of the defined variables and functions leak into global scope. Put them all inside the IIFE to avoid the scope leak: (Note the trailing semicolon)

(function(){
    var _monthList = ...
    function createYears() {
        ...
    }

    ...

    createYears();
    createMonths();
    createDates();
    registerEvents();
})();

And it's better to execute this function only when the document is ready, so it works regardless of when the script is loaded:

$(function() {
    ...
});

Naming

• Don't prefix your variable names with an underscore. This convention is only used for private properties of objects, which you don't have in your code.

• Use day instead of date when you mean the day of the month (e.g. createDays, days, #cbDay, day), Javascript's Date#getDate method is poorly named. Then you can rename dateObj to date.

• Don't prefix your elements' IDs with cb (they're not callbacks). Simply use year, month and day (element IDs don't clash with variable/function names, so they don't need to be prefixed).

• lblError -> errorLabel. No need to abbreviate.

• dataset -> options, to be more descriptive.

Make each function do one thing

createYears both generates the year array and manipulates the DOM. Generate the array outside the function instead: (like you did for createMonths)

var yearList = (function() {
    var years = [];
    for (var i = 2000; i<=new Date().getFullYear(); i++){
        years.push({id:i, name:i});
    }
    return years;
})();

Do the same for createDays.

validateDate both checks if the date is valid and updates #errorLabel accordingly. Use a seperate function for testing if a date is valid:

function isDateValid(day, month, year) {
    var date = new Date(year, month-1, day);
    return day == date.getDate();
}

createOptions

Every time createOptions is called, the return value is set as the HTML value of an element. It's better to pass the element to createOptions as an argument and let it manipulate the element to eliminate this repetition.

Each option passed to this function is known to have id and name properties, so you don't need to check which properties are present.

You don't need to set the IDs for the options, it's enough to set the values. This means the id property of the options in the array argument should be renamed to value (Change id to value when generating the arrays).

Finally, you can use jQuery for creating the elements:

function createOptions(select, options) {
    options.forEach(function (option) {
        select.append($('', { text: option.name, value: option.value }));
    });
}

Now createYears looks like this:

function createYears() {
    createOptions($('#year'), yearList);
}

Which is so short that there is no need to make createYears, createMonths and createDays multiple functions.

Seperating logic from DOM

Put all the functions that are responsible for the logic first, and after them put code that accesses the page's DOM: (i.e. code that has strings with specific element IDs)

var dayList = ...;
var monthList = ...;
var yearList = ...;

function createOptions(select, options){
    ...
}

function isDateValid(day, month, year) {
    ...
}

createOptions($('#year'), yearList);
createOptions($('#month'), monthList);
createOptions($('#day'), dayList);

$("select").on("change", function validateDate() {
    var day = $("#day").val(); /* NOTE: No need to use additional selectors when you're already selecting by ID */
    var month = $("#month").val();
    var year = $("#year").val();
    if(year && month && day) {
        /* NOTE: I used a ternary expression to shorten the code */
        $("#errorLabel").text(isDateValid(day, month, year) ? "" : "Please select a valid date");
    }
});

This makes the code easier to understand and more reusable (e.g. it will make having multiple date forms on the same page easier).

Good luck!

StackExchange Code Review Q#109765, answer score: 2