patternjavascriptMinor
Stack Exchange moderation-action auto-comments
Viewed 0 times
stackexchangeautoactionmoderationcomments
Problem
I've written a userscript that pops up a dialog before you take a moderation action (closing, deleting) with some pro-forma comments, so that you can add one of those before you cast your vote.
I'm pretty happy with how I wrote it, but what's clear to me is unlikely to be so to others. Is there anything in this that I could have "phrased" better, or that could be optimized more? Comments on style (or, for that matter, anything else) are also welcome. It also occurs to me that this isn't as extensible as it could be: it's not easy to add new comments. Is there anything I could do to facilitate this?
I've also put this code in a gist, if you want to see it there.
```
$(document).ready(function() {
// This section is only not in "Program", below, because fail-fast.
if(!StackExchange) {
console.error("SE.MAAC can't run: no StackExchange global");
return;
}
// Utility functions: 'library'-type methods for helping those that perform the major tasks.
function loadDialog(html_or_element, type) {
if(type === "html") {
$("body").loadPopup({
"lightbox": false,
"html": html_or_element,
"target": $("body")
});
}
else if(type === "element") {
if($(html_or_element).length == 1) {
$("body").loadPopup({
"lightbox": false,
"html": $("html_or_element").html(),
"target": $("body")
});
}
else {
throw new ValueError("loadDialog: param 'html_or_element': element does not exist in document tree, or there is more than one matching element.");
}
}
else {
throw new ValueError("loadDialog: param 'type': Value must be one of 'html' or 'element'.");
}
}
function generateDialogContent(title, commentList) {
var contentString = '' + title + ''
I'm pretty happy with how I wrote it, but what's clear to me is unlikely to be so to others. Is there anything in this that I could have "phrased" better, or that could be optimized more? Comments on style (or, for that matter, anything else) are also welcome. It also occurs to me that this isn't as extensible as it could be: it's not easy to add new comments. Is there anything I could do to facilitate this?
I've also put this code in a gist, if you want to see it there.
```
$(document).ready(function() {
// This section is only not in "Program", below, because fail-fast.
if(!StackExchange) {
console.error("SE.MAAC can't run: no StackExchange global");
return;
}
// Utility functions: 'library'-type methods for helping those that perform the major tasks.
function loadDialog(html_or_element, type) {
if(type === "html") {
$("body").loadPopup({
"lightbox": false,
"html": html_or_element,
"target": $("body")
});
}
else if(type === "element") {
if($(html_or_element).length == 1) {
$("body").loadPopup({
"lightbox": false,
"html": $("html_or_element").html(),
"target": $("body")
});
}
else {
throw new ValueError("loadDialog: param 'html_or_element': element does not exist in document tree, or there is more than one matching element.");
}
}
else {
throw new ValueError("loadDialog: param 'type': Value must be one of 'html' or 'element'.");
}
}
function generateDialogContent(title, commentList) {
var contentString = '' + title + ''
Solution
Early returns
The top function is the only place you have used an early return. You could do the same to make
Manual HTML construction
It will be hard to maintain afterward. Have you considered a library like Mustache for this purpose?
Everything else
This is an obvious code duplication:
Could be rewritten as:
I would call
This code doesn't do anything:
The top function is the only place you have used an early return. You could do the same to make
loadDialog and handleDialogResult less hairy.Manual HTML construction
It will be hard to maintain afterward. Have you considered a library like Mustache for this purpose?
Everything else
This is an obvious code duplication:
if(type === "html") {
$("body").loadPopup({
"lightbox": false,
"html": html_or_element,
"target": $("body")
});
}
else if(type === "element") {
if($(html_or_element).length == 1) {
$("body").loadPopup({
"lightbox": false,
"html": $("html_or_element").html(),
"target": $("body")
});
}
// ...
}Could be rewritten as:
var html = type === "html"
? html_or_element
: $("html_or_element").html();
$("body").loadPopup({
"lightbox": false,
"html": html,
"target": $("body")
});I would call
console.warn here:else {
/* Actually throwing an error here would cause more trouble than the semantic improvement is worth *
* because it would halt data processing for every data set. Instead, just forget this data set as *
* invalid and move on. */
continue;
}This code doesn't do anything:
else {
// See notes about continuing in the last else block.
continue;
}addComment function can be shorter if you return a promise:function addComment(postId, commentText) {
return $.ajax({
type: 'POST',
url: '/posts/' + postId + '/comments',
data: {
'comment': commentText,
'fkey': StackExchange.options.user.fkey
}
});
}addComment(postId, comment)
.done(function () {
closeDialogs(".autocomment-dialog");
})
.fail(function (jqXHR) {
alert("Failed to comment: " + jqXHR.responseText);
})
.always(function (_, textStatus) {
console.log(textStatus, jqXHR.responseText);
});
Code Snippets
if(type === "html") {
$("body").loadPopup({
"lightbox": false,
"html": html_or_element,
"target": $("body")
});
}
else if(type === "element") {
if($(html_or_element).length == 1) {
$("body").loadPopup({
"lightbox": false,
"html": $("html_or_element").html(),
"target": $("body")
});
}
// ...
}var html = type === "html"
? html_or_element
: $("html_or_element").html();
$("body").loadPopup({
"lightbox": false,
"html": html,
"target": $("body")
});else {
/* Actually throwing an error here would cause more trouble than the semantic improvement is worth *
* because it would halt data processing for every data set. Instead, just forget this data set as *
* invalid and move on. */
continue;
}else {
// See notes about continuing in the last else block.
continue;
}function addComment(postId, commentText) {
return $.ajax({
type: 'POST',
url: '/posts/' + postId + '/comments',
data: {
'comment': commentText,
'fkey': StackExchange.options.user.fkey
}
});
}Context
StackExchange Code Review Q#112370, answer score: 3
Revisions (0)
No revisions yet.