Bookmarklet that helps Stack Exchange users farm reputation

Question

I made a bookmarklet called RepAdvisor (more info at this link) that rates Stack Overflow questions based on how easy it is to gain reputation by answering them.

(function(){
    var currentDateTime = new Date();
    var epochSeconds = Math.round(currentDateTime.getTime() / 1000);

    function limit(input, limit) {
        return ((input > limit) ? (limit) : (input));
    }

    function transformQuestionScore(input) {
        return limit(input, 15) * 0.01485;
    }
    function transformQuestionAskerRep(input) {
        return limit(input, 750) * 0.00002664;
    }
    function transformQuestionViewVelocity(input) {
        return limit(input, 0.01) * 405.8;
    }
    function transformQuestionViews(input) {
        return Math.pow(0.025 * limit(input, 5000), 0.5);
    }

    function appendHTML(id, content) {
        $("#question-summary-" + id).append("<div style='clear: both;'><strong>" + content + "</strong></div>");
    }

    function zeroToOneHundredScore(number, min, max) {
        var lowerDifference = number - min;
        var minnedMax = max - min;
        var ratio = lowerDifference / minnedMax;
        if (ratio > 1) {
            ratio = 1;
        }
        else if (ratio < 0) {
            ratio = 0;
        }
        return ratio * 100;
    }

    function calculateSumAnswerScore(data) {
        var sumAnswerScore = 0;
        $.each(data.answers, function(index, value) {
            sumAnswerScore = sumAnswerScore + value["score"];
        });
        //var deltaTimeLastActivity = epochSeconds - data["creation_date"];

        var noZeroSumAnswerScoreXXX = ((sumAnswerScore <= 0) ? (0) : (sumAnswerScore));

        return noZeroSumAnswerScoreXXX;
    }

    function calculateCompetitionRating(data) {
        //var answerCount = data["score"];
        //var acceptedAnswer = data["owner"]["reputation"];
        //var views = data["view_count"];
        //var deltaTimeCreation = epochSeconds - data["creation_date"];
        var noZeroSumAnswerScore = calculateSumAnswerScore(data);

        var deltaTimeCreation = epochSeconds - data.question["creation_date"]; // DUPLICATE CODE!
        var deltaTimeLastActivity = epochSeconds - data.question["last_activity_date"];
        var answerCount = data.question["answer_count"];
        var views = data.question["view_count"];
        // // // IMPORTANT // // // var acceptedAnswer = 1;

        var finalScore = (zeroToOneHundredScore(-deltaTimeCreation, -300000, 0) + zeroToOneHundredScore(-deltaTimeLastActivity, -100000, 0) + zeroToOneHundredScore(-answerCount, -2.35, 0) + zeroToOneHundredScore(-noZeroSumAnswerScore, -4, 0) /* * acceptedAnswer * */ + zeroToOneHundredScore(views / deltaTimeCreation, 0, 0.0005)) / 5;



        appendHTML(data.question["question_id"], "Competition Rating: " + Math.round(finalScore));
    }
    function calculateRepProfitEstimate(data) {
        var data_questionScore = data.question["score"];
        var data_askerRep = data.question["owner"]["reputation"];
        var data_views = data.question["view_count"];
        var data_deltaTimeCreation = epochSeconds - data.question["creation_date"]; // DUPLICATE CODE!

        var averageAnswerScore = calculateSumAnswerScore(data) / data.question["answer_count"]; // ANOTHA DUPE

        var rawRepProfit = (transformQuestionScore(data_questionScore) + transformQuestionAskerRep(data_askerRep) + transformQuestionViewVelocity(data_views / data_deltaTimeCreation) + transformQuestionViews(data_views) + limit(averageAnswerScore, 20)) / 4 * 10;
        var data_acceptRate = ((data.question["owner"].hasOwnProperty("accept_rate")) ? (data.question["owner"]["accept_rate"]) : (50));
        rawRepProfit = rawRepProfit + (data_acceptRate / 100) * 15;



        appendHTML(data.question["question_id"], "Estimated Reputation Profit: " + Math.round(rawRepProfit) + ((rawRepProfit >= 90) ? " or more" : ""));
    }

    var fullDataSet = [];

    var questionsToQuery = [];

    function addQandAtoDataSet(question, answers) {
        fullDataSet.push({
            "question" : question,
            "answers" : answers
        });
    }

    function transferJSONtoDataSet(questionsData, answersData) {
        $.each(questionsToQuery, function(index1, value1) {
            var temporaryArray = [];
            $.each(answersData["items"], function(index2, value2) {
                if (value2["question_id"] == value1) {
                    temporaryArray.push(value2);
                }
            });
            $.each(questionsData["items"], function(index3, value3) {
                if (value3["question_id"] == value1) {
                    addQandAtoDataSet(value3, temporaryArray);
                }
            });
        });

    }

    function synthesizeAllData() {
        $.each(fullDataSet, function(index, value) {
            calculateRepProfitEstimate(value);
            calculateCompetitionRating(value);
        });
    }














    var $questionsOnPage = $(".question-summary");
    $questionsOnPage.each(function (index) {
        var $currentQuestion = $(this);
        var questionID = $currentQuestion.attr("id");
        questionID = questionID.substr(questionID.lastIndexOf("-") + 1); // The id of a question is at the end of the question's id attribute. This gets the id from the id.
        questionsToQuery.push(questionID);
    });

    $.get(
        "https://api.stackexchange.com/2.2/questions/" + questionsToQuery.join(";")
        , {
            "order": "desc",
            "sort": "activity",
            "site": "stackoverflow"
        }
        , function (data1) {
            $.get(
                "https://api.stackexchange.com/2.2/questions/" + questionsToQuery.join(";") + "/answers"
                , {
                    "order": "desc",
                    "sort": "activity",
                    "site": "stackoverflow"
                }
                , function (data2) {
                    transferJSONtoDataSet(data1, data2);
                    synthesizeAllData();
                },
                "json"
            );
        },
        "json"
    );
})();

Is my code good? What changes should I make to improve it?

Answer 1

First, a few general pointers.

• Commented out code is bad. It tells readers that you don't really know why this code was there, but removing it seems to have fixed something, or maybe adding it broke something.
• When accessing object properties, avoid using obj["prop"] if possible. Just use obj.prop.
• Try to choose meaningful names, naming things is one of the hardest things to do, but a descriptive name can go a long way towards making code more readable. I'll try to provide some advice on a case-by-case basis when going through the code.

---

Working through this bit by bit, I have next to no experience with jQuery so will only skim those parts. I wrote most of this before Vogel posted an answer, so there may be some duplication here.

First, getting epochSeconds.

var currentDateTime = new Date();
var epochSeconds = Math.round(currentDateTime.getTime() / 1000);

There's no need to create a new Date() to get the current epoch time, I would also prefer Math.floor to Math.round to avoid getting a future time (even though that future time is very close).

var epochSeconds = Math.floor(Date.now())

It is very easy to forget that this variable exists later on in the code, and it really isn't that expensive to recreate later on, so I would recommend either moving this into the function that needs it, or creating a function to get the current time if it is used in multiple functions. Declaring epochSeconds up here also introduces a bug if it takes more than 1 second to fetch question/answer info as the time calculated from "now" may end up being negative.

---

As @Tamoghna Chowdhury said in the comments, your limit function is already built in to the Math object. You can use Math.max and Math.min to get the maximum and minimum values of the passed arguments. Another advantage to using Math.max and Math.min is that the code will be easier to read. If limit is seen out of context, it could easily be assumed that it returns either the maximum or the minimum value.

---

Skipping down to zeroToOneHundredScore, I wish I could suggest a clearer name as the name threw me off for a bit, but I can't seem to come up with anything that I would claim is better at representing the function's purpose without an accompanying comment. Personally, I would likely name this function grade or score and add a JSDoc comment that clarifies that it returns a number between 0 and 100.

I would personally slightly change how this method is written. First, check for the special cases. There is no need to calculate the ratio first as you know that if (number < min) then 0 should be returned, and if (number > max) then 100 should be returned. By checking for the special cases first, the method becomes easier for someone who has no idea what it does to grasp quickly.

function zeroToOneHundredScore(number, min, max) {
    if (number < min) {
        return 0;
    }
    if (number > max) {
        return 100;
    }

    var lowerDifference = number - min;
    var minnedMax = max - min;
    var ratio = lowerDifference / minnedMax;
    return ratio * 100;
}

I believe this is simpler, but still had to pause in order to decipher what lowerDifference and minnedMax were. I can't seem to come up with a good name for lowerDifference, but I believe that score may be slightly better. minnedMax is easier. max - min is equal to the range between max and min.

function zeroToOneHundredScore(number, min, max) {
    if (number < min) {
        return 0;
    }
    if (number > max) {
        return 100;
    }

    var score = number - min;
    var range = max - min;
    var ratio = score / range;
    return ratio * 100;
}

---

Next up, calculateSumAnswerScore. What is sumAnswerScore? It turns out that it is the sum of the scores of all the answers for a given question, but this was not immediately obvious from the function name. I would recommend calculateTotalAnswerScore (or Reputation / Rep instead of Score). calculateSum sounds redundant to me. Another alternative name could be sumAnswerScores (I think this is probably better as it indicates that there are multiple answers)

var sumAnswerScore = 0;
$.each(data.answers, function(index, value) {
    sumAnswerScore = sumAnswerScore + value["score"];
});

This is a perfect place to use Array.prototype.reduce. Here is an ES5 version with the same result:

var score = data.answers.reduce(function(carry, answer) {
    return carry + answer.score;
}, 0);

Or, if you prefer, use arrow functions to slightly reduce the amount of code.

var score = data.answers.reduce((carry, answer) => carry + answer.score, 0);

var noZeroSumAnswerScoreXXX = ... - This needs a better name. XXX provides no value to anyone reading the code and we already know we are talking about the sum of the answer score from the function name. A better name would just be positiveScore or clampedScore as this score also cannot be less than zero. Alternatively, just remove this variable entirely and return the result of the ternary operation. The ternary operation also uses too many parenthesis. At the very least, remove the parenthesis surrounding the entire expression and the two results to make it easier to read.

With these changes made, here's how I would write the method using the ES5 reduce for maximum compatibility.

function sumAnswerScores(question) {
    var score = question.answers.reduce(function(carry, answer) {
        return carry + answer.score;
    }, 0);
    return Math.min(0, score);
}

---

Next up, calculateCompetitionRating, good function name! I know exactly what this method will do. The biggest problem this function has is not the line labeled "duplicate code". It is the massive formula assigned to finalScore. What happens when/if you add the acceptedAnswer calculation back into the score calculation and forget to change 5 to 6? This is a good opportunity to create a helper method to take the average of several numbers.

function mean(numbers) {
    return numbers.reduce(function(carry, num) {
       return carry + num; 
    }, 0) / numbers.length;
}

With this helper function, the function can be simplified slightly.

function calculateCompetitionRating(data) {
    var noZeroSumAnswerScore = calculateSumAnswerScore(data);
    var deltaTimeCreation = epochSeconds - data.question.creation_date;
    var deltaTimeLastActivity = epochSeconds - data.question.last_activity_date;
    var answerCount = data.question.answer_count;
    var views = data.question.view_count;

    var finalScore = mean([
        zeroToOneHundredScore(-deltaTimeCreation, -300000, 0) + 
        zeroToOneHundredScore(-deltaTimeLastActivity, -100000, 0) + 
        zeroToOneHundredScore(-answerCount, -2.35, 0) + 
        zeroToOneHundredScore(-noZeroSumAnswerScore, -4, 0) + 
        zeroToOneHundredScore(views / deltaTimeCreation, 0, 0.0005)
    ])

    appendHTML(data.question.question_id, "Competition Rating: " + Math.round(finalScore));
}

I still believe this can can be improved. The answerCount and view_count variables really don't add anything to the clarity of this function besides providing a shortcut for use later. As the shortcut is only used once, there isn't much point in creating it. On the other hand, data.question does appear several times and can be simplified with a shortcut to question.

function calculateCompetitionRating(data) {
    var question = data.question;
    var totalAnswerScores = sumAnswerScores(data);
    var deltaTimeCreation = epochSeconds - question.creation_date;
    var deltaTimeLastActivity = epochSeconds - question.last_activity_date;

    var score = mean([
        zeroToOneHundredScore(-deltaTimeCreation, -300000, 0),
        zeroToOneHundredScore(-deltaTimeLastActivity, -100000, 0),
        zeroToOneHundredScore(-question.answer_count, -2.35, 0),
        zeroToOneHundredScore(-noZeroSumAnswerScore, -4, 0),
        zeroToOneHundredScore(question.view_count / deltaTimeCreation, 0, 0.0005)
    ]);

    appendHTML(question.question_id, "Competition Rating: " + Math.round(score));
}

Feedback for calculateRepProfitEstimate would essentially be identical to calculateCompetitionRating so I'll leave refactoring this function to you. I will also skip addQandAtoDataSet as I'm going to recommend removing it.

The transferJSONtoDataSet, synthesizeAllData functions are best looked at after reviewing the last bit of code in the IIFE.

var $questionsOnPage = $(".question-summary");
$questionsOnPage.each(function (index) {
    var $currentQuestion = $(this);
    var questionID = $currentQuestion.attr("id");
    questionID = questionID.substr(questionID.lastIndexOf("-") + 1); // The id of a question is at the end of the question's id attribute. This gets the id from the id.
    questionsToQuery.push(questionID);
});

Currently, this code relies on the questionsToQuery variable which is defined over 30 lines earlier with 3 functions in between. This isn't a very good idea as it results in a lot of searching to figure out where questionsToQuery came from. It would be better to just get an array of question ids and assign the result immediately. Thankfully, jQuery can help with this as it provides a map function on the object returned by $().

var questionsToQuery = $(".question-summary").map(function(index, question) {
    // Note: `question` is the element itself, not a jQuery wrapper.
    var id = question.id;
    id = id.substr(id.lastIndexOf('-') + 1);
    return id;
}).get();

Now that we have the IDs, there's no need to wait for the api to return the question data before fetching the answer data. This is a great place to use promises. As the code currently uses $.get I'll continue using it, but I would recommend looking into the built in Fetch API. I'm not sure why you are requiring an order / sort in the API request as you then disregard this information, but I'll leave that in for now as there may be API specific requirements I am unaware of.

// Perform the requests at the same time, resolve when both requests resolve
Promise.all([
    $.get("https://api.stackexchange.com/2.2/questions/" + questionsToQuery.join(";"), {
        order: 'desc',
        sort: 'activity',
        site: 'stackoverflow'
    }).promise(),
    $.get("https://api.stackexchange.com/2.2/questions/" + questionsToQuery.join(";") + "/answers", {
        order: 'desc',
        sort: 'activity',
        site: 'stackoverflow'
    }).promise()
]).then(function(data) {
    transferJSONtoDataSet(data[0], data[1]);
    synthesizeAllData();
});

Now back to transferJSONtoDataSet, first up, there's no need to wrap so many loops! If you loop through everything in questionsData.items you will have looped through all questions. In addition to this, looping through the answers can be simplified by using the Array.prototype.filter method.

function transferJSONtoDataSet(questions, answers) {
    questions.items.forEach(function(question) {
        var thisQuestionsAnswers = answers.items.filter(function(answer) {
            return answer.question_id == question.question_id;
        });

        addQandAtoDataSet(question, thisQuestionsAnswers);
    });
}

Not that that has been cleaned up a bit, I don't see much value in keeping addQandAtoDataSet as all it does is push to an array. The same goes for synthesizeAllData. We can just call calculateRepProfitEstimate and calculateCompetitionRating here. Since the function no longer transfers any data, it's time to rename it to something more descriptive, I choose scoreQuestions, though there is arguably a better name.

function scoreQuestions(questions, answers) {
    questions.items.forEach(function(question) {
        var thisQuestionsAnswers = answers.items.filter(function(answer) {
            return answer.question_id == question.question_id;
        });

        var data = {
            question: question,
            answers: thisQuestionsAnswers
        };
        calculateRepProfitEstimate(data);
        calculateCompetitionRating(data);
    });
}

All together now. This could still be improved. What I would do next is try to only pass each function the data that it requires. For example sumAnswerScores only needs an array of answers, not a wrapper containing both the question and the answers.

(function(){
    "use strict";

    function getEpoch() {
        return Math.floor(Date.now() / 1000);
    }

    function transformQuestionScore(input) {
        return Math.min(input, 15) * 0.01485;
    }
    function transformQuestionAskerRep(input) {
        return Math.min(input, 750) * 0.00002664;
    }
    function transformQuestionViewVelocity(input) {
        return Math.min(input, 0.01) * 405.8;
    }
    function transformQuestionViews(input) {
        return Math.pow(0.025 * Math.min(input, 5000), 0.5);
    }

    function appendHTML(id, content) {
        $("#question-summary-" + id).append("<div style='clear: both;'><strong>" + content + "</strong></div>");
    }

    function zeroToOneHundredScore(number, min, max) {
        if (number < min) {
            return 0;
        }
        if (number > max) {
            return 100;
        }

        var score = number - min;
        var range = max - min;
        var ratio = score / range;
        return ratio * 100;
    }

    function sumAnswerScores(question) {
        var score = question.answers.reduce(function (carry, answer) {
            return carry + answer.score;
        }, 0);
        return Math.max(0, score);
    }

    function mean(numbers) {
        return numbers.reduce(function (carry, num) {
            return carry + num;
        }, 0) / numbers.length;
    }

    function calculateCompetitionRating(data) {
        var question = data.question;
        var totalAnswerScores = sumAnswerScores(data);
        var deltaTimeCreation = getEpoch() - question.creation_date;
        var deltaTimeLastActivity = getEpoch() - question.last_activity_date;

        var score = mean([
            zeroToOneHundredScore(-deltaTimeCreation, -300000, 0),
            zeroToOneHundredScore(-deltaTimeLastActivity, -100000, 0),
            zeroToOneHundredScore(-question.answer_count, -2.35, 0),
            zeroToOneHundredScore(-totalAnswerScores, -4, 0),
            zeroToOneHundredScore(question.view_count / deltaTimeCreation, 0, 0.0005)
        ]);

        appendHTML(question.question_id, "Competition Rating: " + Math.round(score));
    }

    function calculateRepProfitEstimate(data) {
        var question = data.question;
        var views = question.view_count;
        var deltaTimeCreation = getEpoch() - question.creation_date;
        var averageAnswerScore = sumAnswerScores(data) / question.answer_count;

        var rawRepProfit = mean([
            transformQuestionScore(question.score),
            transformQuestionAskerRep(question.owner.reputation),
            transformQuestionViewVelocity(views / deltaTimeCreation),
            transformQuestionViews(views),
            Math.min(averageAnswerScore, 20)
        ]) * 15;

        var owner = question.owner;
        var acceptRate = owner.hasOwnProperty("accept_rate") ? owner.accept_rate : 50;
        rawRepProfit += (acceptRate / 100) * 15;

        appendHTML(question.question_id, "Estimated Reputation Profit: " + Math.round(rawRepProfit) + ((rawRepProfit >= 90) ? " or more" : ""));
    }

    function scoreQuestions(questions, answers) {
        questions.items.forEach(function (question) {
            var thisQuestionsAnswers = answers.items.filter(function (answer) {
                return answer.question_id == question.question_id;
            });

            var data = {
                question: question,
                answers: thisQuestionsAnswers
            };
            calculateRepProfitEstimate(data);
            calculateCompetitionRating(data);
        });
    }

    var questionsToQuery = Array.from(document.querySelectorAll(".question-summary")).map(function(question) {
        var id = question.id;
        id = id.substr(id.lastIndexOf("-") + 1);
        return id;
    });

    // Perform the requests at the same time, resolve when both requests resolve
    Promise.all([
        $.get("https://api.stackexchange.com/2.2/questions/" + questionsToQuery.join(";"), {
            order: 'desc',
            sort: 'activity',
            site: 'stackoverflow'
        }).promise(),
        $.get("https://api.stackexchange.com/2.2/questions/" + questionsToQuery.join(";") + "/answers", {
            order: 'desc',
            sort: 'activity',
            site: 'stackoverflow'
        }).promise()
    ]).then(function(data) {
        scoreQuestions(data[0], data[1]);
    });

})();

Answer 2

Skimming from top to bottom, just dumping what comes to mind here:

• You should be using 'use strict'; when you can, to make your code conform to modern standards. Since it's not nice to force it unto others though, you should scope it to your top-level IIFE:

  (function () {
      'use strict';
      // ...
  })();
  

• limit is more generally called min, which is already implemented. You already use Math.round, why not Math.min?

• All your transform* functions extensively use magic numbers. You should extract these to named constants.

• Math.pow([..], 0.5) is generally easier to understand as Math.sqrt([..])

• zeroToOneHundredScore should be named something with percent. Also it'd probably make calling the function easier if you provided nice defaults for min and max

• Why are you using the .each polyfill for summing over an array? Just be explicit and use the following:

  var sumAnswerScore = 0;
  for (var answer of data.answers) {
      sumAnswerScore += answer["score"];
  }
  

• remove commented out code. it's just confusing to keep it in the "production" code

• noZeroSumAnswerScoreXXX is a confusing name... what does the XXX mean? Also that could be simplified and cleaned up as follows:

  // clamp sum to a minimum of 0
  return Math.max(0, sumAnswerScore); 
  

• The calculation of finalScore is an extremely long line riddled with magic numbers again... What do the numbers mean, why is this formula the way it is? How can we tweak it?

• Why does calculateCompetitionRating call appendHTML?

• Similar considerations as for the finalScore computation apply for rawRepProfit and data_acceptRate

• There's a huge gap of empty space between synthesizeAllData and var $questionsOnPage... why? On that note: why did you prefix these variables with $? Is it because these are JQuery objects? because if that's the case, you should consider dumping JQuery and using vanilla js to get the questionsToQuery from the page...