HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Stack Exchange moderation-action auto-comments

Submitted by: @import:stackexchange-codereview··
0
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 + ''

Solution

Early returns

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.