patternjavascriptMinor
jQuery Upvote, a simple Stack Exchange style voting plugin
Viewed 0 times
simplevotingstackexchangestylejquerypluginupvote
Problem
I'm not very good at JavaScript, so it would be great if somebody could review my jQuery Upvote plugin, and point out mistakes, bad practices, or anything suspicious.
The repository is on GitHub. Here's a live demo, and qunit tests, if you're interested, but I'm not asking for a review of those.
Here's the code, with the header omitted (full version here):
```
;(function($) {
"use strict";
var namespace = 'upvote';
var dot_namespace = '.' + namespace;
var upvote_css = 'upvote';
var dot_upvote_css = '.' + upvote_css;
var upvoted_css = 'upvote-on';
var dot_upvoted_css = '.' + upvoted_css;
var downvote_css = 'downvote';
var dot_downvote_css = '.' + downvote_css;
var downvoted_css = 'downvote-on';
var dot_downvoted_css = '.' + downvoted_css;
var star_css = 'star';
var dot_star_css = '.' + star_css;
var starred_css = 'star-on';
var dot_starred_css = '.' + starred_css;
var count_css = 'count';
var dot_count_css = '.' + count_css;
var enabled_css = 'upvote-enabled';
function init(options) {
return this.each(function() {
methods.destroy.call(this);
var count = parseInt($(this).find(dot_count_css).text());
count = isNaN(count) ? 0 : count;
var initial = {
id: $(this).attr('data-id'),
count: count,
upvoted: $(this).find(dot_upvoted_css).size(),
downvoted: $(this).find(dot_downvoted_css).size(),
starred: $(this).find(dot_starred_css).size(),
callback: function() {}
};
var data = $.extend(initial, options);
if (data.upvoted && data.downvoted) {
data.downvoted = false;
}
var that = $(this);
that.data(namespace, data);
render(that);
setupUI(that);
});
}
function setupUI(that) {
that.find(dot_upvote_css).addClass(en
The repository is on GitHub. Here's a live demo, and qunit tests, if you're interested, but I'm not asking for a review of those.
Here's the code, with the header omitted (full version here):
```
;(function($) {
"use strict";
var namespace = 'upvote';
var dot_namespace = '.' + namespace;
var upvote_css = 'upvote';
var dot_upvote_css = '.' + upvote_css;
var upvoted_css = 'upvote-on';
var dot_upvoted_css = '.' + upvoted_css;
var downvote_css = 'downvote';
var dot_downvote_css = '.' + downvote_css;
var downvoted_css = 'downvote-on';
var dot_downvoted_css = '.' + downvoted_css;
var star_css = 'star';
var dot_star_css = '.' + star_css;
var starred_css = 'star-on';
var dot_starred_css = '.' + starred_css;
var count_css = 'count';
var dot_count_css = '.' + count_css;
var enabled_css = 'upvote-enabled';
function init(options) {
return this.each(function() {
methods.destroy.call(this);
var count = parseInt($(this).find(dot_count_css).text());
count = isNaN(count) ? 0 : count;
var initial = {
id: $(this).attr('data-id'),
count: count,
upvoted: $(this).find(dot_upvoted_css).size(),
downvoted: $(this).find(dot_downvoted_css).size(),
starred: $(this).find(dot_starred_css).size(),
callback: function() {}
};
var data = $.extend(initial, options);
if (data.upvoted && data.downvoted) {
data.downvoted = false;
}
var that = $(this);
that.data(namespace, data);
render(that);
setupUI(that);
});
}
function setupUI(that) {
that.find(dot_upvote_css).addClass(en
Solution
This in JSHint
JSHint says "Possible strict violation" because you are using
In non-strict mode, calling
If that's how you intend to implement, you can ignore JSHint, as you will not generate any errors. But, it is telling you that your code is unclear to anyone reading it, because using
See the explanation in this SO Answer.
Also your missing a radix parameter in parseInt in line 25 presumably you want the number in base 10, so the line should be:
It'll work fine either way, but you said
JSHint says "Possible strict violation" because you are using
this inside something that, as far as it can tell, is not a method.In non-strict mode, calling
_click_downvote(5) would bind this to the global object (window in the browser). In strict mode, this would be undefined, and you would get in trouble. Binding it as a listener in jQuery would make this the target of the event.If that's how you intend to implement, you can ignore JSHint, as you will not generate any errors. But, it is telling you that your code is unclear to anyone reading it, because using
this inside of something that is not obviously a method is quite confusing. It would be better to simply pass the object as a parameter.See the explanation in this SO Answer.
Also your missing a radix parameter in parseInt in line 25 presumably you want the number in base 10, so the line should be:
var count = parseInt($(this).find(dot_count_css).text(),10);It'll work fine either way, but you said
'use strict'. :)Code Snippets
var count = parseInt($(this).find(dot_count_css).text(),10);Context
StackExchange Code Review Q#49231, answer score: 6
Revisions (0)
No revisions yet.