Calculate LIX value of a text

Question

I've been building a simple tool to calculate the LIX value of a text (a standard measurement of the readability of a text).

My approach was to have every step of the calculation as seperate functions. In a larger and more complex tool the total words, sentences and others would be nice to have as independent functions.

Also the functions should be able to take in any string input and do their calculations. So that I would easily be able to use this in other contexts like TinyMCE editor and so forth.

So I've tried to find a way to make the code flexible. And my ambition was to write simple and pure functions.

I'm a little in doubt as to how readable and maintainable this approach is.

The click function triggering the calculation is an example. I'd really like your thoughts about this functions inside functions approach. I found it very nice to work in this way.

jQuery(document).ready(function() {


  function getText() {
    var text = jQuery('#inputLix').val();

    return text;
  }

  function countWords(input) {
    var wordCount;
    var wordsArray;
    var splitter = /\s|\n/g;
    wordsArray = input
      .replace(/(<([^>]+)>)/ig, "")
      .replace(/,/g, "")
      .replace(/-/g, "")
      .replace(/"/g, "")
      .replace(/'/g, "")
      .split(splitter)
      .filter(Boolean);
    wordCount = wordsArray.length;
    return wordCount;
  }


  function longWordCount(input) {
    var longWordCounter = 0;
    var splitter = /\s|\n/g;
    var checkedText = input.replace(/<([^>]+)>|[\.\+\?,\(\)"'\-\!\;\:]/ig, "");
    var wordsArray = checkedText
      .split(splitter)
      .filter(Boolean);

    for (var i in wordsArray) {
      if (wordsArray[i].charAt(0) > -1) {} else if (wordsArray[i].length > 6) {
        longWordCounter++;
      }
    }
    return longWordCounter;
  }


  function getSentences(input) {
    var splitter = /\?|\!|\.|\n/g;
    var checkedText = input
      .replace(/(<([^>]+)>)/ig, "")
      .replace(/,/g, "")
      .replace(/-/g, "")
      .replace(/"/g, "")
      .replace(/'/g, "");
    var arrayOfSentences = checkedText.split(splitter);
    arrayOfSentences = arrayOfSentences.filter(Boolean);
    return arrayOfSentences;
  }

  function countSentences(input) {
    return input.length;
  }

  function calculateLix(wordCount, sentenceCount, longWordsCount) {
    var lixScore = Math.round((wordCount / sentenceCount) + ((longWordsCount * 100) / wordCount));
    return lixScore;
  }

  function outputContent(input, id) {
    jQuery(id).html(input);
  }


  jQuery("#lixButton").click(function() {
    var input = getText();
    outputContent(calculateLix(countWords(input), countSentences(getSentences(input)), longWordCount(input)), "#notification");
  });

});

.tool-wrapper {
  background: #899398;
  padding: 30px;
  border-radius: 10px;
}

textarea {
  width: 100%;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="tool-wrapper">
  <textarea id="inputLix" rows="15">This is just some basic text. Push the button to calculate the LIX value. And figure out how hard your text is to read.</textarea>
  <div class="clearfix">
    <button id="lixButton" class="alignright" type="submit">Calculate Lix</button>
    <div id="notification" class="alignright">You haven't performed any calculations yet</div>
  </div>
</div>

Answer 1

Your code could be simplified and better organized.

• You seem to be expecting HTML, as evidenced by various calls that look like .replace(/(<([^>]+)>)/ig, ""). However, that is a half-assed way of interpreting HTML as text.

  Consider that your getSentences() treats a newline as a possible sentence delimiter. But all whitespace in HTML is considered equivalent, more or less. If your input is indeed HTML, you should normalize all whitespace as spaces, and interpret tags such as <p> and <br> as line breaks.

  Also, if the input is HTML, you ought to decode entities (&65; is A, for example).

• Most of your functions have parameters named input. Well, duh. All parameters act as input, by definition. But what kind of input? Here is the most baffling example, where the input is supposed to be an array of sentences:

  function countSentences(input) {
    return input.length;
  }
  

  If the input is HTML text, call it html. If it's plain text, then call it text.

• The click handler is a bit convoluted. It should look like this:

  jQuery("#lixButton").click(function() {
    var html = jQuery('#inputLix').val();
    jQuery('#notification').text(lixScore(htmlToText(html)));
  });
  

  In particular, your getText() function hard-coded the ID of the textarea, which I don't recommend. Your outputContent(), in contrast, does take a jQuery selector as a parameter (inappropriately named id), which is good, but needlessly complicated. I would eliminate both getText() and outputContent().

• The calculateLix() function is hard to use. Why does it need so many parameters? Why not just feed it some text and have it do the analysis?

• The helper functions are inconsistently designed: countWords(input) vs. longWordCount(input) vs. countSentences(getSentences(input)).

  The implementations of countWords() and longWordCount() overlap, and they treat punctuation differently (why?). Once you have identified the words, shouldn't counting long words be a simple matter of filtering by length?

  Why does getSentences() mess around with punctuation that is irrelevant to sentence-splitting?

Suggested implementation

jQuery(document).ready(function() {
  var htmlNamedEntities = {
    "amp": "&",
    "quot": '"',
    "lt": "<",
    "gt": ">"
  };

  function htmlToText(html) {
    return html.replace(/\s+/, " ")
               .replace(/<\/?\s*(p|br|hr)[^>]*>/ig, "\n")
               .replace(/(<[^>]+>)|&(\d+);|&x([0-9a-f]+);|&([a-z]+);/ig,
      function($0, tag, decimalEntity, hexEntity, namedEntity) {
        return tag ? ""
             : decimalEntity ? String.fromCharCode(parseInt(decimalEntity))
             : hexEntity ? String.fromCharCode(parseInt(hexEntity, 16))
             : htmlNamedEntities[namedEntity];
      }
    );
  }

  function words(text) {
    return text.replace(/[-']/ig, "")  // Ignore hyphens and apostrophes.
               .split(/\W+/)           // This assumes that the text is alphanumeric English.
               .filter(Boolean);
  }

  function sentences(text) {
    return text.split(/[?!.\n]+/)
               .filter(Boolean);
  }

  function lixScore(text) {
    var textWords = words(text);
    var w = textWords.length;
    var lw = textWords.filter(function(w) { return w.length > 6 }).length;
    var s = sentences(text).length;
    return Math.round((w / s) + (100 * lw / w));
  }

  jQuery("#lixButton").click(function() {
    var html = jQuery('#inputLix').val();
    jQuery('#notification').text(lixScore(htmlToText(html)));
  });

});

.tool-wrapper {
  background: #899398;
  padding: 30px;
  border-radius: 10px;
}

textarea {
  width: 100%;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="tool-wrapper">
  <textarea id="inputLix" rows="5">This is just some basic text. Push the button to calculate the LIX value. And figure out how hard your text is to read.</textarea>
  <div class="clearfix">
    <button id="lixButton" class="alignright" type="submit">Calculate Lix</button>
    <div id="notification" class="alignright">You haven't performed any calculations yet</div>
  </div>
</div>

Answer 2

Removing symbols and splitting words

The replacements here can lead to words getting stuck For example if a space character is missing after a comma or a double-quote, the two words before and after become one, incorrectly.

wordsArray = input
  .replace(/(<([^>]+)>)/ig, "")
  .replace(/,/g, "")
  .replace(/-/g, "")
  .replace(/"/g, "")
  .replace(/'/g, "")
  .split(splitter)
  .filter(Boolean);

You can make it more robust by replacing comma and double-quote with a space instead. The case of dash and apostrophe is not so obvious, and it depends on your requirements. For example, is "well-known" one word or two? If one, then removing dash is correct. Otherwise you should replace with a space instead.

The purpose of .filter(Boolean) here is to remove empty strings. A better way to achieve that would be to make the splitter span all the whitespace in between words, for example /\s+/. Note that you will still need the .filter(Boolean), to handle the case when the input is empty. But the solution will be slightly more efficient, as the array result of the split will not need to be mutated, except in the case of empty input.

I recommend to write like this:

  function countWords(input) {
    return input
      .replace(/<[^>]+>|['-]/g, "")
      .replace(/[,"]/g, " ")
      .split(/\s+/)
      .filter(Boolean)
      .length;
  }

I removed all the local variables, as they were not very useful, and also simplified the regular expressions.

Answer 3

The cost of software

In my opinion the code you have presented is overly complex and extremely resource wasteful. This is the type of code that gets sent back for rewrite.

I understand you are still learning, and apart from the two bugs your code does its job. But there is more to coding than just creating working software.

You write for the client first

As a developer you must provide the best possible product for the client, the community, and the developer as a business.

Software incurs costs for the client and community in terms of both time and power. This can be measured by CPU cycles and memory usage.

Costs to the developer are in terms of deployment (bandwidth cost) and negative association (Developer X's code is slow and power hungry).

Bugs

The code you created also has a number of bugs.

• You can get incorrect sentence counts because you count /n as a sentence delimiter. "Blah blah. Foo\nbar.".split(/\?|\!|\.|\n/g).length returns the wrong count.
• Long word count throws in some environments as you do not check for own property in the iterator.

Bad code

The following bit of code is wrong in many ways.

for (var i in wordsArray) {
  if (wordsArray[i].charAt(0) > -1) {} else if (wordsArray[i].length > 6) {
    longWordCounter++;
  }
}

Problems

• See second bug in bugs above. You should use wordsArray.hasOwnProperty(i) to make sure you are not iterating an inherited property.

• Never create a unused condition. Should be if ( ! wordsArray[i].charAt(0) > -1 && wordsArray[i].length > 6) { longWordCounter++; }

• What are you doing String.prototype.charAt(index) returns a string. Are you trying to exclude words with leading numbers??? If so you should remove them much earlier.

• It is just bad practice to use for ... in ... on arrays. Use an iterator or a for or while loop.

The following is wasteful in both CPU and Memory and also should use appropriate variables.

  function longWordCount(input) {
    var longWordCounter = 0;
    var splitter = /\s|\n/g;
    var checkedText = input.replace(/<([^>]+)>|[\.\+\?,\(\)"'\-\!\;\:]/ig, "");
    var wordsArray = checkedText
      .split(splitter)
      .filter(Boolean);

    for (var i in wordsArray) {
      if (wordsArray[i].charAt(0) > -1) {} else if (wordsArray[i].length > 6) {
        longWordCounter++;
      }
    }
    return longWordCounter;
  }

• The split creates an array. The filter creates another array, this means you have iterated the array of words twice before you even start counting. In the meantime you have two copies of the words in memory (one waiting for GC) the other that you iterate over for the 3rd time.

• The variables splitter, checkedText, wordArray, and i are all constants and should be declared as such using the const token.

• The RegExp flag i is redundant and should not be included.

JQuery

Do you really need jQuery? I ask this question alot and the correct answer is not a flat "yes". You must justify the cost of all that extra code and why alternative solutions are not better.

Rewrite

I had to rewrite. The following does the same, in a fraction of the time and a fraction of the memory. It is a saving of cost in all areas.

function calculateLIX(text) {
    function filterText(text) {
        return text.replace(/<([^>]+)>|[\+,\(\)"'\-\;\:]/g, "")
            .replace(/\t|\n|\r/g, " ")
            .replace(/ +/g, " ")
            .trim();
    }
    function longWords(text) {
        var count = 0;
        text.replace(/\.|\!|\?/g, "") // remove sentence ends
            .replace(/\S+/g, word => (count += word.length > 6 ? 1 : 0, ""));
        return count;
    }
    text = filterText(text);
    const LIX = {
        counts : {
            words : text.replace(/[^ ]/g, "").length + 1,
            longWords : longWords(text),
            sentences : text.replace(/[^\.\?\!]/g, "").length,
        },
        toString() {
            return "LIX score : " + LIX.score
        },
    };
    LIX.score = Math.round((LIX.counts.words / LIX.counts.sentences) + ((LIX.counts.longWords * 100) / LIX.counts.words));
    return LIX;
}
const LIXAnalysis = calculateLIX(document.getElementById(textElementId).value);
document.querySelector("#notification").textContent = LIXAnalysis.toString();

Answer 4

The functions look nicely organized to me, modulo missing helper functions. Your click function certainly makes sense.

Nit: getText is accurate but on the vague side. Maybe call it getInputText? It's trivial enough to maybe not warrant being a separate function, that sort of hinges on whether the name offers valuable documentation.

In countWords you might combine assignment with var declarations, as you did with splitter.

regexes

This line:

  .replace(/(<([^>]+)>)/ig, "")

suggests you're (sometimes) feeding raw HTML as input text. HTML is famously "too complex" for regex parsing. I recommend you use $ lynx -dump or similar tool to convert to text prior to the Lix evaluation.

Perhaps I'm missing something here:

var splitter = /\s|\n/g;

Newline characters were matched already by \s, weren't they?

You could turn four calls into one:

  .replace(/[,"'-]/g, "")

BTW, it seems like there's lots more punctuation that might go in there, but perhaps that suffices for your needs. Beware the em-dash and the dreaded smart quote.

Overall countWords seems slightly complex for what it does. Might be able to get away with just a splitter that looks for /[a-zA-Z0-9]+/, at least for ASCII text. Use character classes if e.g. vowels with accent marks matter.

In longWordCount, checkedText is a bit horrible. Now, maybe it needs to be, and there's no getting around it. But I'm suggesting factoring it out as a helper function, so that the definition of what a "word" is will be consistent across this function, and countWords, and others.

I note that European punctuation and others are missing from checkedText, but perhaps that's enough for your needs. The (roughly) \w+ splitter I suggested above is one way to avoid having to enumerate all punctuation marks.

Copy-n-pasting splitter is a code smell here, which reinforces the "break out a helper method" suggestion.

style

Break if (wordsArray[i].charAt(0) > -1) {} else if ... into multiple lines. Better, alter the booleans to remove that empty statement (unless you wanted to insert some comment describing the empty statement case). I personally don't understand the test, but then I come from Python land where it's all unicode. Maybe it's testing for ASCII 128 .. 255?

Certainly getSentences needs a comment describing how we define a sentence. Wrapped text with newlines every 70 characters would have a surprisingly high number of short sentences. Again we see U.S.-centric text processing, which is fine, but there should be an overall comment at top of file describing such assumptions, and also describing utf8 vs. other text encodings being the expected input.

It's not clear countSentences has a reason for being.

If countWords was named getWords, you might use getWords().length in the same way as getSentences().length.

The call to outputContent() would benefit from putting arguments on multiple lines.

corner cases

Maybe it's impossible for sentenceCount & wordCount to be zero. I just worry about div-by-zero ugliness.

Answer 5

Thanks for all the great and detailed feedback. A lot of great input. I've tried to rewrite the JS based on your different input.

I ditched jQuery, ditched the for loop and much more.

I ditched jQuery and tried to rewrite based on your input. Here is the new version:

document.addEventListener("DOMContentLoaded", function() {
    'use strict';

    function words(text) {
        return text
            .replace(/[-'.]/ig, "") // Ignores hyphens and apostrophes. Dots are here to avoid split on . in numbers.
            .split(/[^a-zA-ZæøåÆØÅ0-9]/g) // Can't use \W+ since I need to take into account danish character ÆØÅ
            .filter(Boolean);
    }

    function sentences(text) {
        var splitter = /\?|\!|\.|\n/g;
        var arrayOfSentences = text
            .split(splitter)
            .filter(Boolean);
        return arrayOfSentences;
    }

    function calculateLix(text) {
        var wordCount = words(text).length;
        var longWordsCount = words(text)
            .filter(function(wordsArray) { return wordsArray.length > 6; })
        	.length;
        var sentenceCount = sentences(text).length;
        var lixScore = Math.round((wordCount / sentenceCount) + ((longWordsCount * 100) / wordCount));
        return lixScore;
    }

    document.getElementById('lixButton').addEventListener('click', function() {
        var t0 = performance.now();
        var text = document.getElementById('inputLix').value;
        document.querySelector('#notification').textContent = calculateLix(text).toString();
        var t1 = performance.now();
        console.log('Took', (t1 - t0).toFixed(4), 'milliseconds');
    });
});

.tool-wrapper {
  background: #899398;
  padding: 30px;
  border-radius: 10px;
}

textarea {
  width: 100%;
}

<div class="tool-wrapper">
  <textarea id="inputLix" rows="15">This is just some basic text. Push the button to calculate the LIX value. And figure out how hard your text is to read.</textarea>
  <div class="clearfix">
    <button id="lixButton" class="alignright" type="submit">Calculate Lix</button>
    <div id="notification" class="alignright">You haven't performed any calculations yet</div>
  </div>
</div>