Make a summary from a larger text-file

Question

This code makes summaries from larger texts.

I have searched around for an algorithm and found the following:

1. Associate words with their grammatical counterparts. (e.g. "city" and "cities")

2. Calculate the occurrence of each word in the text.

3. Assign each word with points depending on their popularity.

4. Detect which periods represent the end of a sentence. (e.g "Mr." does not).

5. Split up the text into individual sentences.

6. Rank sentences by the sum of their words' points.

7. Return X of the most highly ranked sentences in chronological order.

Based on this, I've written some code.

I'm fairly new to javascript so basically I'd really appreciate tips on the following:

• Possible improvements on algorithm
• Coding style

summary.js

var stopwords = require('stopwords').english;     //https://github.com/huned/node-stopwords
var natural   = require('natural');               //https://github.com/NaturalNode/natural


function Summary(text) {

    if (typeof text !== 'string') throw new Error("Argument must be a string");

    this.text = text
    this.sentences = [];
    this.sentencesScore = {};
    this.wordPoints = {};
}

Summary.prototype.wordFrequency = function () {

    return this.text.split(/\s+/)
                    .filter(word => { return stopwords.indexOf(word.toLowerCase()) < 0; })
                    .map(word => { return natural.PorterStemmer.stem(word); })
                    .reduce(function(map, word) {

                        map[word] = (map[word] || 0) + 1;
                        return map;

                     }, this.wordPoints);
}


Summary.prototype.splitTextIntoSentences = function () {
    this.sentences = this.text.replace(/([.?!])\s*(?=[A-Z])/g, "$1|").split("|")

    return this.sentences;
}

Summary.prototype.rankSentences = function () {
    for(var i = 0, count = 0; i < this.sentences.length; i++) {
        var sentence = this.sentences[i];

        sentence.split(/\s+/)
                .filter(word => { return stopwords.indexOf(word.toLowerCase()) < 0; })
                .map(word => { return natural.PorterStemmer.stem(word); })
                .forEach(word => {
                    count += this.wordPoints[word] || 0;
                 });

        this.sentencesScore[sentence] = count; 
        count = 0;
    }

}

Summary.prototype.printMostValuable = function() {
    var sortable = [];

    for (var sentence in this.sentencesScore) {
        sortable.push([sentence, this.sentencesScore[sentence]])
    }
    sortable.sort(function(a, b) {return b[1] - a[1]});

    console.log(sortable);
}




var text = "The political future of Brazilian President Dilma Rousseff hangs in the balance as lawmakers consider a request to impeach her and she waits to hear whether the country's Supreme Federal Court will deal her a significant setback.The court is considering whether to allow Rousseff's appointment of ex-President Luiz Inacio Lula da Silva to her Cabinet, a move that would help shield him from prosecution while he is under investigation in a wide-ranging graft probe. The court made no decision when it reconvened Wednesday, but it was scheduled to meet again Thursday.Rousseff is also facing maneuvers in the National Congress to impeach her over allegations that she tried to hide a budget shortfall ahead of elections in 2014.Hundreds of government supporters carrying red banners bearing the logo of her ruling Workers' Party gathered on the lawn Thursday before the Congress in the capital, Brasilia, where the impeachment commission was hearing the testimony of government witnesses.Supporters plan to stage demonstrations in five states across the country Thursday in solidarity with the embattled President."

var s = new Summary(text);

s.wordFrequency();
s.splitTextIntoSentences();
s.rankSentences();  

s.printMostValuable();

The output from this, is an array containing the sentences + the calculated score (most important):

[ [ 'Hundreds of government supporters carrying red banners bearing the logo of her ruling Workers\' Party gathered on the lawn Thursday before the Congress in the capital, Brasilia, where the impeachment commission was hearing the testimony of government witnesses.',
    28 ],
  [ 'The political future of Brazilian President Dilma Rousseff hangs in the balance as lawmakers consider a request to impeach her and she waits to hear whether the country\'s Supreme Federal Court will deal her a significant setback.',
    24 ],
  [ 'The court is considering whether to allow Rousseff\'s appointment of ex-President Luiz Inacio Lula da Silva to her Cabinet, a move that would help shield him from prosecution while he is under investigation in a wide-ranging graft probe.',
    19 ],
  [ 'Rousseff is also facing maneuvers in the National Congress to impeach her over allegations that she tried to hide a budget shortfall ahead of elections in 2014.',
    14 ],
  [ 'Supporters plan to stage demonstrations in five states across the country Thursday in solidarity with the embattled President.',
    11 ],
  [ 'The court made no decision when it reconvened Wednesday, but it was scheduled to meet again Thursday.',
    8 ] ]

Answer 1

Use a linter

The first thing I do when reviewing JavaScript code is run it through a linter. There several online linters, JSHint for example. JSHint will complain about ES6 features:

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

To get around this, add this comment line at the top:

// jshint esversion: 6

It's recommended to clear out all the reported warnings.

I will address the more interesting ones in sub-sections below.

"Don't make functions within a loop"

In the rankSentences function, you use lambda expressions in a loop:

for(var i = 0, count = 0; i < this.sentences.length; i++) {
    var sentence = this.sentences[i];

    sentence.split(/\s+/)
            .filter(word => { return stopwords.indexOf(word.toLowerCase()) < 0; })
            .map(word => { return natural.PorterStemmer.stem(word); })
            .forEach(word => {
                count += this.wordPoints[word] || 0;
             });

    this.sentencesScore[sentence] = count; 
    count = 0;
}

A lambda expression is a function, so effectively you're redefining the same function repeatedly, in each iteration. This is akin to computing the same value repeatedly. The solution is to define the function once before the loop, which is akin to moving a computation into a local variable before the loop begins:

function isStopWord(word) {
    return stopwords.indexOf(word.toLowerCase()) < 0;
}

function setBaseWord(word) {
    return natural.PorterStemmer.stem(word);
}

for(var i = 0, count = 0; i < this.sentences.length; i++) {
    var sentence = this.sentences[i];

    sentence.split(/\s+/)
              .filter(isStopWord)
              .map(setBaseWord)
              .forEach(word => {
                  count += this.wordPoints[word] || 0;
               });

    this.sentencesScore[sentence] = count; 
    count = 0;
}

"The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype."

This answer on Stack Overflow explains well the problem and the solution for this warning:

What does the JSLint error 'body of a for in should be wrapped in an if statement' mean?

Don't repeat yourself

Notice that the functions we extracted in isStopWord and setBaseWord are also used in wordFrequency. So these functions could be defined in the global namespace and reused.

You could go even further. It could be a good idea to move the entire sentence.split(...).filter(...).map(...) to a common function, as this entire snippet appears twice.

Another example where you repeated yourself is the use of the count variable in the rankSentences function: it's set to 0 in the for system, and then again at the end of the loop body. It would be better to set it to 0 once, at the beginning of the loop body:

for(var i = 0; i < this.sentences.length; i++) {
    var sentence = this.sentences[i];
    var count = 0;

    sentence.split(/\s+/)
              .filter(isStopWord)
              .map(setBaseWord)
              .forEach(word => {
                  count += this.wordPoints[word] || 0;
               });

    this.sentencesScore[sentence] = count; 
}

Performance

Note that stopwords.indexOf is an \$O(N)\$ operation. If performance is important, you could build a map, to make this operation \$O(1)\$, at the expense of using more memory (the storage of the map).