HTML scraping and using jQuery

I wrote a Chrome extension that lists upcoming concerts for the next seven days, this month, and next month. I had trouble with the nextSeven() method, especially transitioning from dates like Dec. 31st to Jan. 1st, hence the really fugly if-statements. I was wondering if anyone had any advice to make the code more condensed. I would really appreciate any feedback!



`$(document).ready(function() {

var months = ['Jan. ', 'Feb. ', 'March ', 'April ', 'May ', 'June ', 'July ', 'Aug. ', 'Sept. ', 'Oct. ', 'Nov. ', 'Dec. '];
var systemTime = new Date();

$.ajax({
url: 'http://nycmetalscene.com/',
type: 'GET',
success: function(data) {
var parsedData = $.parseHTML(data);
var parseResults = $(parsedData).find('p').text();
var events = parseResults.split(/Sun\. |Mon\. |Tues\. |Wed\. |Thurs\. |Fri\. |Sat\. /g);
events.splice(0, 1);
var monthIndex = systemTime.getMonth();
var day = systemTime.getDate();
var testDay = systemTime.getDate();
var week = [];
var thisDay;

getDays();

$('#nextseven').on('click', function() {
$('#concerts').empty();
nextSeven();
});

$('#thismonth').on('click', function() {
$('#concerts').empty();
showMonth();
});

$('#nextmonth').on('click', function() {
$('#concerts').empty();
showNextMonth();
});

function getDays() {

for (var i = 0; i = 26) {
if (eventsstr.match(months[monthIndex]) || eventsstr.match(months[0])) {
var concert = JSON.parse(eventsstr);
$('#concerts').append('
' + concert + '');
}

} else if (monthIndex = 26) {
if (eventsstr.match(months[monthIndex]) || eventsstr.match(months[monthIndex + 1])) {
var concert = JSON.parse(eventsstr);
$('#concerts').append('
' + concert + '');
}

} else {

What doesn't work properly

There are two severe issues in the way you evaluate when to go from a given month to the next one.

When defining the set of days in a week

var systemTime = new Date();
var day = systemTime.getDate();
// ...
for (var i = 0; i < 7; i++) {
  if (day < 31) {
    week.push(day);
    day++;

  } else {
    week.push(day);
    day = 1;
  }
}

In the above code you work as if all months had 31 days!

Instead you should take account of those having only 30 days, and of February with its 28 or 29 days depending on bisextile years.

Obviously doing so becomes a much more complex task, so I'd suggest using a totally different way, based on the Date object internal capabilities:

var systemTime = new Date();
// ...
for (var i = 0; i < 7; i++) {
  var day = systemTime.getDate();
  week.push(day);
  systemTime.setDate(day + 1);
}

This way, not only we ensure to get correct days in any case but the code is even more reduced than before.

When looking for an involved month in the content of an event

The same kind of error appears in the nextSeven() function, when you test monthIndex = 26: in fact, testing begDay >= 26 is appropriate only for months having 31 days... you know the next.

But there is also another issue here:

• for each event you select it if it's matching thisDay

• then you look for the given monthIndex or monthIndex + 1, independently from the current day

• so you may accept wrong days, having the required number but belonging to another month!

Again I'll suggest to work based on full dates rather than separate days and months, but it'll be more clear to explain it later (see "The week case" below).
How to simplify and reduce code

Your code can be improved in several ways.

Formatting day

Once defined the set of days, here is how you finally format them:

for (var i = 0; i < week.length; i++) {
  if (week[i] === 1 || week[i] === 31) {
    week[i] = ' ' + week[i] + 'st,';

  } else if (week[i] === 2 || week[i] === 22) {
    week[i] = ' ' + week[i] + 'nd,';

  } else if (week[i] === 3 || week[i] === 23) {
    week[i] = ' ' + week[i] + 'rd,';

  } else {
    week[i] = ' ' + week[i] + 'th,';
  }
}

First I'd suggest using map() instead of a for() loop.

A second improvement is to factorize what's common to all cases, starting by defining the proper suffix, then building the result at once.
And finally define suffix in a more compact way, for example using a predefined object registering suffixes.

This way, the whole code sequence above can be replaced by this:

const suffixes = {'1': 'st', '2': 'nd', '3': 'rd'};
var systemTime = new Date();
// ...
week = week.map(function (day) {
  return ' ' + day + (suffixes[day % 10] || 'th') + ',';
});

Defining days at once

In fact, we now see that the two partial tasks examined above can be joined, so the whole getDays() function becomes much more simple:

const suffixes = {'1': 'st', '2': 'nd', '3': 'rd'};
var systemTime = new Date();
// ...
function getDays() {
  for (var i = 0; i < 7; i++) {
    day = systemTime.getDate();
    week.push(' ' + day + (suffixes[day % 10] || 'th') + ',');
    systemTime.setDate(day + 1);
  }
}

Reorganization

It must be observed that this getDays() function only serves to populate weeks, which itself is used only in the nextSeven() context.

From that:

• The getDays(); call should'nt appear in the general context of success but in its own nextSeven()context.

• In this context, we see that the whole job is wrapped in a for() loop iterating week, to work on thisDay, which is merely week[i].

• So it'll be more efficient to abandon the getDays() function and integrate its job to nextSeven().

Current organization schema:

getDays();
// ...
function getDays() {
  for (var i = 0; i < 7; i++) {
    var day = systemTime.getDate();
    week.push(' ' + day + (suffixes[day % 10] || 'th') + ',');
    systemTime.setDate(day + 1);
  }
}
// ...
function nextSeven() {
  for (i = 0; i < week.length; i++) {
    thisDay = week[i];
    // ...
  }
}

Suggested organization schema:

function nextSeven() {
  for (var i = 0; i < 7; i++) {
    var day = systemTime.getDate();
    thisDay = ' ' + day + (suffixes[day % 10] || 'th') + ',';
    // ...
    systemTime.setDate(day + 1);
  }
}

(note: this way, the week variable doesn't exist any more)

Compacting some sequences

There are cases where using several statements to build a desired result can be simplified to a unique statement, so avoiding temporary variables not used elsewhere, though becoming more readable at the same time.

At the opposite, yet for readability, it may be better to extract constants from the sequence.

Here is an example:

var parsedData = $.parseHTML(data);
var parseResults = $(parsedData).find('p').text();
var events = parseResults.split(/Sun\. |Mon\. |Tues\. |Wed\. |Thurs\. |Fri\. |Sat\. /g);
events.splice(0, 1);

It can be replaced

StackExchange Code Review Q#149453, answer score: 3