3
\$\begingroup\$

I created some functionality for JavaScript datepicker.

I'm trying to store a value in the datepicker with the selected month along with which half it is. To do this, I wrote if/else statements to account for every month.

Among the rules I noted were:

  • 28 days - February
  • 30 days - April, June, September, November
  • 31 days - January, March, May, July, August, October, December

I'm left with some horribly inefficient code:

$('.datepicker').click(function() {
    var months = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"];
  var date = $('.datepicker').val();
var firstChar = date.match('[a-zA-Z]');
    var index = date.indexOf(firstChar);

if ((Number(date.slice(3,5)) < 15) || (index === 0)) {
      if (Number(date.slice(0,2)) === 1 || date.indexOf("Jan") === 0) {
        $(this).val(months[0] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 2 || date.indexOf("Feb") === 0) {
          $(this).val(months[1] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 3 || date.indexOf("Mar") === 0) {
          $(this).val(months[2] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 4 || date.indexOf("Apr") === 0) {
          $(this).val(months[3] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 5 || date.indexOf("May") === 0) {
          $(this).val(months[4] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 6 || date.indexOf("Jun") === 0) {
          $(this).val(months[5] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 7 || date.indexOf("Jul") === 0) {
          $(this).val(months[6] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 8 || date.indexOf("Aug") === 0) {
          $(this).val(months[7] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 9 || date.indexOf("Sep") === 0) {
          $(this).val(months[8] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 10 || date.indexOf("Oct") === 0) {
          $(this).val(months[9] + "(1-15)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2) === 11) || date.indexOf("Nov") === 0) {
          $(this).val(months[10] + "(1-15)" + "," + new Date().getFullYear());
        }
        else {
          $(this).val(months[11] + "(1-15)" + "," + new Date().getFullYear());
        }
    }
    else {
      if (Number(date.slice(0,2)) === 2 || date.indexOf("Feb") === 0) {
        $(this).val(months[1] + "(16-28)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 4 || date.indexOf("Apr") === 0) {
          $(this).val(months[3] + "(16-30)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 6 || date.indexOf("Jun") === 0) {
          $(this).val(months[5] + "(16-30)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 9 || date.indexOf("Sep") === 0) {
          $(this).val(months[8] + "(16-30)" + "," + new Date().getFullYear());
        }
        else if (Number(date.slice(0,2)) === 11 || date.indexOf("Nov") === 0) {
          $(this).val(months[10] + "(16-30)" + "," + new Date().getFullYear());
        }

        else {

            if (Number(date.slice(0,2)) === 1 || date.indexOf("Jan") === 0) {
                $(this).val(months[0] + "(16-31)" + "," + new Date().getFullYear());
            }
            else if (Number(date.slice(0,2)) === 3 || date.indexOf("Mar") === 0) {
                $(this).val(months[2] + "(16-31)" + "," + new Date().getFullYear());
            }
            else if (Number(date.slice(0,2)) === 5 || date.indexOf("May") === 0) {
                $(this).val(months[4] + "(16-31)" + "," + new Date().getFullYear());
            }
            else if (Number(date.slice(0,2)) === 7 || date.indexOf("Jul") === 0) {
                $(this).val(months[6] + "(16-31)" + "," + new Date().getFullYear());
            }
            else if (Number(date.slice(0,2)) === 8 || date.indexOf("Aug") === 0) {
                $(this).val(months[7] + "(16-31)" + "," + new Date().getFullYear());
            }
            else if (Number(date.slice(0,2)) === 8 || date.indexOf("Oct") === 0) {
                $(this).val(months[9] + "(16-31)" + "," + new Date().getFullYear());
            }
          else {
              $(this).val(months[11] + "(16-31)" + "," + new Date().getFullYear());
            }
        }
    }

});

I'm trying to store a value based on the time range a date falls into. I ended up having to manually enter every condition. Is there a way to make this more efficient?

I thought about using a for loop, but I think I have too much going on. Not only do I have to account for the variable i but also of the individual month abbreviations (jan, feb, etc.)

\$\endgroup\$
1
\$\begingroup\$

Naming of variables:

index tells you nothing about what that variable does. firstCharIndex or something like that would be better. But you only care if a character is the first character. So probably use

var usingAbbrFormat = (date.indexOf(firstChar) === 0);

Then when you check whether this is true it is immediately obvious what is going on. I was pretty confused at what this was doing at first.

And once I did figure out what you were doing it made me aware that there was some faulty logic. Your first if clause says IF dayOfMonth < 15 OR usingAbbrFormat. So if you are using the abbreviation format you will never have a second half entry.

Now to your real question:

For the first half of the month you have this. I'm just showing the first one but they are all the same.

if (Number(date.slice(0,2)) === 1 || date.indexOf("Jan") === 0) {
    $(this).val(months[0] + "(1-15)" + "," + new Date().getFullYear());
}

The simplest way to remove the repeating is as you thought. Use a for loop. But since you need to test for a month abbreviation you need that info to test against.

var monthAbbrs = ["Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oc", "Nov", "Dec"];

Now you can do this:

for (i = 0; i < months.length; i++) {
    if (Number(date.slice(0,2)) === (i + 1) || date.indexOf(monthAbbrs[i]) === 0) {
        $(this).val(months[i] + "(1-15)" + "," + new Date().getFullYear());
    }
}

The second half is a little tougher. You can create other arrays to test against for number of days in the month.

var monthsWith31Days = [1, 3, 5, 7, 8, 10];

for (i = 0; i < months.length; i++) {
    if (Number(date.slice(0,2)) === (i + 1) || date.indexOf(monthAbbrs[i]) === 0) {

        if (monthWith31Days.indexOf(i+1)) {
            halfMonth = "(16-31)";
        } else if (i == 1) {
            halfMonth = "(16-28)";
        } else {
            halfMonth = "(16-30)";
        }

        $(this).val(months[i] + halfMonth + "," + new Date().getFullYear());


    }
}

Now since I think you have an error in your logic anyway in your first if statement I'm going to assume it is not needed and remove it. Then we can just combine everything into the below:

var monthsWith31Days = [1, 3, 5, 7, 8, 10];

for (i = 0; i < months.length; i++) {
    if (Number(date.slice(0,2)) === (i + 1) || date.indexOf(monthAbbrs[i]) === 0) {

        if (Number(date.slice(3,5)) < 15) {
            halfMonth = "(1-15)";
        } else if (monthsWith31Days.indexOf(i+1)) {
            halfMonth = "(16-31)";
        } else if (i == 1) {
            halfMonth = "(16-28)";
        } else {
            halfMonth = "(16-30)";
        }

        $(this).val(months[i] + halfMonth + "," + new Date().getFullYear());


    }
}

If you want to make the code even more readable add null as the first item in the month arrays. Then you can do the for loop using 1 as the start point. Makes more sense when dealing with months. for (i=1;i<months.length;i++)

I didn't test any of this so there is probably typos.

EDIT:

I wanted to point out that this won't work if the formatting of your date picker changes. If you are using jQuery datepicker you should look into the api to get the date selected instead of trying to parse the value.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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