6
\$\begingroup\$

For a homework assignment, I made a web page that has a text area. The user inputs some text, hits the analyse button, and the output is supposed to be a list of the frequency of words of a given number of characters.

For example, "one two three" would be two words of three characters and one word of five characters.

It works, but is there room for improvement?

HTML:

<body>
<h3>Text Input</h3>
<textarea id="text" placeholder="type something..." rows="20" cols="80"></textarea>
<button id="analyse">Click to analyze</button>
<h3>Results</h3>
<div id="results"></div>
</body>

JavaScript:

function getWordInfo() { 
    var text1 = document.getElementById("text").value;
    var text2 = text1.replace(/[^\d\w ]+/g," ");
    var text3 = text2.split(/[ ]+/);
    return text3;
}

function firstLoop() {
    var text = getWordInfo();
    var wordMap = [];
    var i =0;
    var looplength = text.length;
    for ( i ;i < looplength; i++) {
        wordMap.push(text[i].length)
    }
    return wordMap;
}

function objCon(name,count) { //object constructor
    this.name = name;
    this.count = count;
}

function maxNum() {
    var array1 = firstLoop();
    var num = Math.max.apply(Math, array1);
    return num;
}

function objArrCon() { //object array constructor 
    var num = maxNum();
    var array1 = [];
    var i = 2;
    for ( i ; i <= num; i++) {
        var myObj = new objCon(i,0);
        array1.push(myObj);
    }
    return array1;
}

function objArrParse() { //updates the object with word info
    var array1 = firstLoop();
    var array2 = objArrCon();
    var loopLength1 = array1.length;
    var loopLength2 = array2.length;
    for (var i = 0; i < loopLength1; i++) {
        for (var m = 0; m < loopLength2; m++) {
            if (array2[m].name === array1[i]) {
                array2[m].count++;
                }
        }
    }
    return array2;
}

function objArrTrun() { //object array truncation 
    var array1 = objArrParse();
    var len = array1.length;
    for (var i = 0;i < len; i++) {
        if (array1[i].count === 0) {
            array1.splice(i, 1);
            len--;
            }
        }
    return array1;
}

function formatter() {
    var strAr = objArrTrun();
    var arrayLength = strAr.length;
    var formatStr = [];
    for (var i = 0; i < arrayLength; i++) {
        var str = "Number of characters: " + strAr[i].name + ", Number of words with this length: " + strAr[i].count;
        formatStr.push(str);
    }
    return formatStr;
}

function clearBox(elementID) {
    document.getElementById(elementID).innerHTML = "";
}

function analyseButtonClick() {
    clearBox("results");
    var html = " ";
    var str = formatter();
    var len = str.length;
    for (var i = 0; i < len; i++) {
        html += "<p>" + str[i] + "<p>";
}
    document.getElementById("results").innerHTML += html;
}

function init() {
    var button = document.getElementById("analyse");
    button.onclick = analyseButtonClick;
}

window.onload = init;
\$\endgroup\$
  • \$\begingroup\$ Quick regexp tips: \w is equivalent to [A-Za-z0-9_] and thus \d is unnecessary. Did you intend to allow words with underscores? Any single-element grouping can be replaced by the element; /[ ]+/ is the same as / +/. \$\endgroup\$ – David Harkness Jun 14 '14 at 20:28
  • \$\begingroup\$ copy/pasted the regex. no clue how they work, just that they do. \$\endgroup\$ – Arron Grier Jun 14 '14 at 20:31
3
\$\begingroup\$

Here's a different approach, using the DOM instead of innerHTML, and using document fragments to append all at once and improve performance. Also you don't need that many regex, specially if you're copy/pasting them; splitting by any whitespace character and removing empty elements from the resulting array will do.

Then you could make use of the builtin higher-order functions such as map and reduce to minimize the use of for loops and make the code a bit cleaner:

var doc = document;

var extend = function(a, b) {
  Object.keys(b).forEach(function(k){a[k] = b[k]});
  return a;
};

var elem = function(tag, props) {
  return extend(doc.createElement(tag), props);
};

/**
 * @param words {Array}
 * @returns {Object} Frequency of words per length
 */
var frequency = function(words) {
  return words.filter(Boolean).reduce(function(acc, x) {
    acc[x.length] = ++acc[x.length] || 1;
    return acc;
  },{});
};

var empty = function(x) {
  while (x.hasChildNodes())
    x.removeChild(x.lastChild);
  return x;
};

/**
 * @param freq {Object} Frequence of words per length
 * @returns {Node} Document fragment
 */
var resultOf = function(freq) {
  return Object.keys(freq).reduce(function(frag, k) {
    frag.appendChild(elem('p', {
      textContent: 'Words with '+ k +' chars: '+ freq[k]
    }));
    return frag;
  }, doc.createDocumentFragment());
};

var textarea = doc.getElementById('text');
var button = doc.getElementById('analyse');
var result = doc.getElementById('results');

button.addEventListener('click', function() {
  var res = resultOf(frequency(textarea.value.split(/\s+/)));
  empty(result).appendChild(res);
});

Demo: http://jsbin.com/motoy/1/edit

To learn more about these idioms, I'd recommend reading JavaScript Allonge, and Functional JavaScript. For JavaScript documentation visit the MDN

\$\endgroup\$
  • \$\begingroup\$ i must admit i am still a big noob when it comes to JavaScript. I could not follow even half of your solution. where could i go to learn these things? my textbook is useless. \$\endgroup\$ – Arron Grier Jun 14 '14 at 21:50
  • \$\begingroup\$ To learn some of these idioms, I'd recommend JavaScript Allonge, and Functional JavaScript. For proper documentation visit the MDN \$\endgroup\$ – elclanrs Jun 14 '14 at 22:00
  • \$\begingroup\$ @elclanrs I would add your comments as a "source" part since it could be useful for future readers! \$\endgroup\$ – Marc-Andre Jun 16 '14 at 2:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.