Various simple JavaScript solutions

Question

Please critique my solutions to these simple exercises. I have no programming experience, so the more anal you are, the more I'll learn. I'm teaching myself, this is not for school:

Write a function translate() that will translate a text into "rövarspråket". That is, double every consonant and place an occurrence of "o" in between. For example, translate("this is fun") should return the string "tothohisos isos fofunon".

var translate = function(x) {
  var string = x.toLowerCase();
  var vowels = ["a", "e", "i", "o", "u", " "];
  var y = "";
  for (i = 0; i < string.length; i++) {
     var current = string.charAt(i); 
    if (vowels.indexOf(current) != -1)
    y = (y + (current));
   else 
    y = (y + (current + "o" + current));
  }
  return y;
}

Define a function sum() and a function multiply() that sums and multiplies (respectively) all the numbers in an array of numbers. For example, sum([1,2,3,4]) should return 10, and multiply([1,2,3,4]) should return 24.

var sum = function(array) {
  var length = array.length;
  var total = 0;
  for (i = 0; i < length; i++) {
    total += array[i];
  }
  return total;
};

var multiply = function(array) {
  var length = array.length;
  var total = 1;
  for (i = 0; i < length;  i++) {
    total *= array[i];
  }
  return total;
};

Define a function reverse() that computes the reversal of a string. For example, reverse("jag testar") should return the string "ratset gaj".

var reverse = function(string) {
  var length = string.length;
  var reversed = [];
  var joined = ("");
  for (i = length; i > 0; i--){
    reversed.push(string.charAt(i-1));
  };
  for (i = 0; i < (length) ; i++){
    joined += (reversed[i]);
  }
  return joined;
}

Write a function findLongestWord() that takes an array of words and returns the length of the longest one.

var findLongestWord = function(array) {
  var elements = array.length;
  var count = 0;
  for (i = 0; i < elements; i++) {
    if (array[i].length > count) 
      count = array[i].length;
  }
  return count;
}

Write a function filterLongWords() that takes an array of words and an integer i and returns the array of words that are longer than i.

var filterLongWords = function(array, int){
  var length = array.length;
  var longestWords = [];
  for (i = 0; i < length; i++) {
    if (array[i].length > int) {
      longestWords[longestWords.length] = array[i];
    }
  }
  return longestWords;
}

Write a function charFreq() that takes a string and builds a frequency listing of the characters contained in it. Represent the frequency listing as a JavaScript object. Try it with something like charFreq("abbabcbdbabdbdbabababcbcbab").

var charFreq = function(string){
  var list = {};
  var length = string.length;
  for (var i = 0; i < length; i++) {  
  if (string.charAt(i) in list) 
    list[string.charAt(i)] += +1;
  else 
    list[string.charAt(i)] = 1;
  }
  return list;
}

EDIT: I forgot to mention that rather than use global methods, I tried to as much as possible code for the result I wanted, like reverse, and join, etc.

Answer 1

I'd start by pointing out that you forgot to declare your loop variables, so they become global, you need var:

for (var i; ...
    --^--

Also you forgot a few semicolons after your function expressions, JSHint will tell you about these common errors.

But your code looks good, although maybe not very JavaScript-ish. Targeting ES5 compliant browsers (IE9+) you could simplify your code, so instead of using for loops, you'd use built-in higher-order functions, such as map, filter and reduce:

var sum = function(xs) {
  return xs.reduce(function(x, y){return x + y});
};

var multiply = function(xs) {
  return xs.reduce(function(x, y){return x * y});
};

var reverse = function(x) {
  return x.split('').reverse().join('');
};

var findLongestWord = function(xs) {
  return Math.max.apply(0, xs.map(function(x){return x.length}));
};

var filterLongWords = function(xs, i) {
  return xs.filter(function(x){return x.length > i});
};

var charFreq = function(x) {
  return x.split('').reduce(function(acc, x) {
    acc[x] = ++acc[x] || 1;
    return acc;
  },{});
};

For code that is very generic I prefer to use short variables names such as xs, x, y, etc. These are inspired by other functional languages, and are well understood, but you could always add JSDoc annotations, for example:

/**
 * @param {String} x A string
 * @returns {Object} Counted occurrences of characters in x
 */
var charFreq = function(x) {
  return x.split('').reduce(function(acc, x) {
    acc[x] = ++acc[x] || 1;
    return acc;
  },{});
};

Answer 2

Here one way to sum a list of integers using ECMAScript 5's functional support

// Sum all values in a list by reducing the list to a single value
array.reduce(function(prev, curr) {
  return prev + curr;
});

Not faster than your approach due to the repeated function calls, but definitely shorter.

Your approach can be further simplified:

// Move everything into the parenthesis of for-loop!
var sum = function(array) {
  for (
    var i = 0, len = array.length, total = 0; 
    i < len; 
    total += array[i++]
  );
  return total;
};

Not recommended because it's not very readable. I just think it's interesting :)

I'll give you an alternative approach to the "Find length of longest string" problem:

// Sort it by length descending and then get the first
var longestStrLen = array.sort(function(a, b) {
    a.length - b.length;
})[0].length;

Hope these open your eyes to some of the possibilities!

Now, here's an actual review:

var charFreq = function(string){
  var list = {}; // Not a list (implies order). Should name it a dictionary or map
  var length = string.length; // no need to have this be separate line
  for (var i = 0; i < length; i++) {  
  if (string.charAt(i) in list) 
    // What's with the +1??
    list[string.charAt(i)] += +1;
  else 
    // No need to call string.charAt multiple times!
    list[string.charAt(i)] = 1;
  }
  return list;
}

Revised version:

var charFreq = function(string){
  var dict = {}; 
  for (var i = 0, len = string.length; i < len; i++) {
    var currChar = string.charAt(i);  
    if (currChar in dict) { 
      dict[currChar] += 1;
    } else {
      dict[currChar] = 1;
    }
  }
  return dict;
}

Hope that helps!

Answer 3

translate() is best done using regular expression substitution. (It's unclear from the problem specification how uppercase inputs should be handled.)

function translate(str) {
    return str.replace(/[^aeiou]/gi, '$1o$1');
}

---

Your sum() and multiply() are fine, except that you forgot to declare var i, so it's global. As a nitpick, I'd rename total → product in multiply(). You could write them more succinctly as

function sum(array) {
    return array.reduce(function(n, total) { return total + n; });
}

function multiply(array) {
    return array.reduce(function(n, product) { return product * n; });
}

However, that's not necessarily better than your original code. For one thing, the one-liner version is about 10× slower.

---

Your reverse() is inefficient. By appending one character at a time to the array, the machine is unable to tell how large the array should be, and therefore may need to silently reallocate the array if it turns out that it needs to be longer than it guessed. By appending one character at a time to the resulting string, you make it take O(n²) time, since each append operation results in a new string, necessitating the entire intermediate string to be copied.

This should be faster. The one-liner by @elclanrs would also be quite good.

function reverse(string) {
    var rev = new Array(string.length);
    for (var i = string.length - 1; i >= 0; i--) {
        rev[i] = string[string.length - i - 1];
    }
    return rev.join('');
}

---

findLongestWord() is misnamed, in my opinion. You want the length of the longest word, not the word itself.

Again, you forgot to declare var i.

I'd prefer a higher-level functional approach, as Math.max() and word.length provide better hints at the purpose of the code, and therefore make it more readable in my opinion. However, your approach isn't bad. Note that my version returns -Infinity for an empty array; if you want 0 then you would have to insert a conditional.

function findLongestWordLength(array) {
    return Math.max.apply(null, array.map(function(word) {
                return word.length;
           }));
}

---

Your charFreq() is fine, but I'd tweak it to be more concise.

function charFreq(string) {
    var freq = {};
    for (var i = string.length - 1; i >= 0; i--) {
        freq[string[i]] = 1 + (freq[string[i]] || 0);
    }
    return freq;
}

Answer 4

Try using '' instead of "" wherever possible. In most languages the content of the double quotes gets interpreted, so the single quote version is faster and does not leak variables.

I'm not sure if it is better, but you can define functions like this too:

function findLongestWord(array) {

instead of

var findLongestWord = function(array) {