Board Thread:Code Review/@comment-4356266-20151008134915/@comment-24473195-20151010095805

Mfaizsyahmi wrote: From the looks of things everyone is so focused on modules, in which case I'd drastically cut back my prospect for a proper code review. Not really. There are more coders using javascript than lua modules.

Doing a code-review is rarely easy,  most people here probably don't know how to do it, and most importantly, mediawiki is awful for code-review. I can't even tell you what lines have problems because the script is in a namepace that automatically wraps everything.

First of all, considering passing it through a tool like jslint (http://www.jslint.com/), it reports some warnings that you may want to look into, especially with your regex.

Anyway, I looked into your code (though I didn't test it), and found that your parse function can be improved:


 * Though you check if the divs in the parse page exist, you don't check if they have the right fields (start_date, end_date, id), e.g. evaluating whether it is a really a date or id using regex.


 * You make excessive use of the $(this) parameter, which makes it hard to review, consider giving it a descriptive variable name.


 * Make sure your regex is valid


 * Don't set attributes without making sure they are what you need (validation)


 * Don't simply set html without escaping it:

var $content = $(' ').addClass('notiplus').attr('id',id).html(html).


 * Put if statement body in a separate line.

Hope that helps.