Matrix transposition of a string Per a Reddit Daily Programming Challenge, I am performing a matrix transposition on a string using JavaScript.

Example input: 'The quick brown fox jumped over the lazy dog'

Output:




Tqbfjotld
hurouvhao
eioxmeezg
 cw pr y
 kn e
    d



While my code works, I wanted to know:


From a readability standpoint, would it be better to break the code up into smaller functions?
If there's anything in the code that can be simplified.




transposeString('The quick brown fox jumped over the lazy dog');

function transposeString(str) {

  if (str.indexOf(' ') === -1) {
    var map = Array.prototype.map;

    var output = map.call(str, function (char) {
        return char + '\n';
    });
    return output.join('');
  } else {
    var master = [],
        longestWord = '';

    words = str.split(' ');

    words.forEach(function(word) {
      if (word.length &gt;= longestWord.length) longestWord = word;
    });

    longestWord.split('').forEach(function() {
      master.push([]);
    });

    var treatedWords = words.map(function (word) {
      if (word.length &lt; longestWord.length) {
        var spaces = Array(longestWord.length - word.length + 1).join(' ');
        return word = word + spaces;
      } else {
        return word;
      }
    });

    treatedWords.forEach(function (word) {
      var n = 0;
      word.split('').forEach(function (char) {
        master[n].push(char);
        n++;
      });
    });

    master.forEach(function (arr) {
      console.log(arr.join(''));
    });

    return;
  }
}
Here you go, with comments.

Obviously, in production I would have only a handful of comments and not the exhaustive ones I put here. However this is the sort of points I might make during a code review.

When I write code I optimize for it for communicating to a human reader what I intend the code to do. In other words - I want it to read like an essay. Therefore, I extract anything detail-heavy into small helper methods at the bottom of the script. The idea is that someone should open my script and the first thing they see should give them a top-level overview of the general flow of logic.

I didn't comment at all on your solution itself, instead just cleaning up the algorithms that were already there.

transposeString('The quick brown fox jumped over the lazy dog');

//Always comment public apis with usage and your intent when writing it.

//Transpose a string of words into a vertical matrix
//from http://www.reddit.com/r/dailyprogrammer/comments/1m1jam/081313_challenge_137_easy_string_transposition/
function transposeString(str) {
  // the ~ (ones complement) operator makes indexOf work like 'contains'
  // ~(-1) == 0, ~(x != -1) != 0
  // not sure why you need this condition at all however in terms of the problem - seems like everything
  //would just work
  if (!~str.indexOf(' ')) 
    //definitely don't have two large branches of conditional logic in the same function, put them 
    //in separate well-named functions
    //Also always prefer to return early to get the simple paths out of the way in conditional logic. 
    //The whole "single point of exit" advice is from back in the day of monolithic functions and gotos all over the place
    return makeVerticalString(str);

  //My preference is to use a single var block with everything aligned and commas preceeding
  //the terms. I have good reasons for this but not anything that I care to get into without
  //being super-pedantic. It's ok to have a different style here.
  var  words            = str.split(' ')         //you were misssing var here
      ,longestWord      = getLongestWord(words)  //give algorithms their own function

      //word matrix - I recommend commenting this so that you know it's a matrix and not a flat array
      //also note that it is much cleaner to use a map than a forEach and explicit push
      //When you see array.push() anywhere it can almost always be cleaned up with a map
      ,transposedMatrix = longestWord.split('').map(function() { return [] })                     

      //give a name to the thing that you're doing here and extract the algorithm to its own function
      //also rename 'treated' to 'padded' to be more explicit
      ,paddedWords = words.map(padWordsShorterThan(longestWord.length));
      ;
  //again, move the actual algorithm to its own labeled function
  paddedWords.forEach(addEachLetterToARow(transposedMatrix));

  transposedMatrix.forEach(function (arr) {
    console.log(arr.join(''));
  });
  //no need for return if you're not returning anything

  //A line to deliniate private helper functions
  //////////////////////////////////////////////////////////////////////////////////////////////
  function makeVerticalString(str) {
    //You're reducing an array to a single value so really reduce is what you want here
    Array.prototype.reduce.call(str, function(m, c) { return m+c+'\n'}, '')
  }

  function getLongestWord(words) {
    //again going from an array to a single value - we can use reduce here
    return words.reduce(function(longest, thisWord){ return thisWord.length &gt;= longest.length ? thisWord : longest}, '');
  }

  //a function that returns a function can be a good tool for controlling scope while keeping things readable
  //note that the inner function still has a name so that it shows up properly in callstack traces  (rather than "anonymous")
  function padWordsShorterThan(threshold) { return function padWordsShorterThan(word) { 
    if (word.length &gt;= threshold)
      return word;        
    //no need for else clauses when you return early
    return word + repeatStr(" ", longestWord.length - word.length + 1);
  }}
  function repeatStr(str, times) {
    //you have a neat little way of implementing a character repeat but its difficult to read - drop it in its own method
    return Array(times).join(str)
  }


 //Naming anonymous functions not only helps make clear what they're doing
 //but makes them show up named in a debugging callstack rather than "anonymous"
 //Rule of thumb - name anonymous functions that are doing significant logic
 function addEachLetterToARow(matrix) { return function addEachLetterToARow(word) {
    var n = 0;
    word.split('').forEach(function addToRow(char) {
      matrix[n].push(char); //note that you have a possible error condition here where matrix[n] does not exist. Do you want to check for that explicitly?
      n+=1; //I agree with Doug Crockford that ++ is wierd and error-prone. += reads much better in my opinion
    });
  } }
}