5
\$\begingroup\$

This question has already got some reviews, so I could partially improve my code: please look at my self-answer below, which also contains comments about the review.


I'm frequently interested to answer SO questions regarding (JS) objects and/or arrays manipulation.

When providing a snippet, and if the actual result looks somewhat complex, it's not enough to merely console.log() it, because it's rendered as a compacted (and often even reduced) single line where it's not easy to directly retrieve each piece of data.

Then I was tired to have to either spend time to arrange the output a bit (but generally not completely) or let the result appear unclear, so I decided to write a helper function for that:

/**
 * @param src: any variable of any type
 * @param html: output format (true|false); default = false
 * @param level: (internal, don't use)
 *
 * @return string: formatted output
 */
function showObj(src, html, level) {
  level |= 0;
  const tabCount = 4,
  edges = '{[}]';
  var tab = (!!html ? ' ' : ' ').repeat(tabCount),
      br = !!html ? '<br />' : '\n',
      offset = tab.repeat(level);
  switch (typeof src) {
    case 'boolean':
    case 'number': return src.valueOf();
    case 'string': return '"' + src.toString() + '"';
    case 'symbol': return src.toString();
    case 'undefined': return 'undefined';
    case 'function': return (src.name || 'function') + '()';
    case 'object':
      if (src === null) {return 'null';}
      if (src === this) {return src.valueOf();}
      if (!!src.jquery) {return 'jQuery ' + src.jquery + ' object';}
      if (!Object.keys(src).length) {return src;}
      var output = [],
          isArr = Array.isArray(src);
      for (let key in src) {
        output.push(
          br + offset + tab + (isArr ? '' : ('"' + key + '": ')) +
          showObj(src[key], html, level + 1)
        );
      }
      return edges[+isArr] + output.join(', ') + br + offset + edges[+isArr + 2];
    default:
      return '(unexpected!) ' + typeof src;
  }
}

/**
 * @param src: any variable of any type
 * @param html: output format (true|false); default = false
 * @param level: (internal, don't use)
 *
 * @return string: formatted output
 */
function showObj(src, html, level) {
  level |= 0;
  const tabCount = 4,
        maxString = 40,
        edges = '{[}]';
  var tab = (!!html ? '&nbsp;' : ' ').repeat(tabCount),
      br = !!html ? '<br />' : '\n',
      offset = tab.repeat(level);
  switch (typeof src) {
    case 'boolean':
    case 'number': return src.valueOf();
    case 'string': return '"' + src.toString().substr(0, maxString) +
      (src.length > maxString ? '...' : '') + '"';
    case 'symbol': return src.toString();
    case 'undefined': return 'undefined';
    case 'function': return (src.name || 'function') + '()';
    case 'object':
      if (src === null) {return 'null';}
      if (src === this) {return src.valueOf();}
      if (!!src.jquery) {return 'jQuery ' + src.jquery + ' object';}
      if (!Object.keys(src).length) {return src;}
      var output = [],
          isArr = Array.isArray(src);
      for (let key in src) {
        output.push(
          br + offset + tab + (isArr ? '' : ('"' + key + '": ')) +
          showObj(src[key], html, level + 1)
        );
      }
      return edges[+isArr] + output.join(', ') + br + offset + edges[+isArr + 2];
    default:
      return '(unexpected!) ' + typeof src;
  }
}

var src = {
  'null': null,
  'undefined': undefined,
  'this': this,
  'integer': 123,
  'float': 12.3,
  'Infinity': Infinity,
  'NaN': NaN,
  'Math.PI': Math.PI,
  'string': 'Some short string',
  'long-string': showObj.toSource(),
  'bool-true': true,
  'bool-false': false,
  'date': new Date(),
  'regexp': /.*/i,
  'error': new Error('Error message'),
  'symbol': Symbol('someSymbol'),
  'function': function someFunc() {},
  'anonymous-func': function() {},
  'simple-array': [1, 2, 3],
  'simple-object': {a: 'A', b: 'B', c: 'C'},
  'compound-object': {
    'compound-array': [
      {'integer': 123, 'float': 12.3},
      {'NaN': NaN, 'string': 'A string'},
      {
        'sub-array': [1, 2, 3],
        'sub-object': {a: 'A', b: 'B', c: 'C'}
      }
    ]
  },
  'jQuery': $('body'),
  'map': new Map([['key1', 'value1'], ['key2', 'value2']]),
  'set': new Set(['value1', 'value2', 'value3'])
};

console.log(showObj(src));
document.write(showObj(src, true));
* {
  font-family: "Courier New";
  font-size: 12px;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

I tried to make it together:

  • Complete, i.e. distinguishing each kind of data structure I could think to (but maybe I forgotten some cases?).
  • Clear but concise, e.g. cutting long strings, omitting to expand this or jQuery objects, and so on.
  • Also with a reduced code (I'm not re-writing a console :).

I'm expecting comments about especially the three aspects above, as well as about anything wrong I might have done.

\$\endgroup\$
  • \$\begingroup\$ What's the meaning of the |= operator in "level |= 0" ? I have never seen that before. \$\endgroup\$ – michael.zech Mar 21 '16 at 7:22
  • \$\begingroup\$ @st88 It's a shortcut for something like (longest form) level =( typeof level == 'undefined') ? 0 : level;. Since it's expected that the most outer call to showObj() doesn't specify any 3rd argument, it works with 0 as value for level. \$\endgroup\$ – cFreed Mar 21 '16 at 14:57
  • \$\begingroup\$ With your description and documentation I could read the code from top to bottom. Understood at once which way things are accomplished. Therefore: Clear? Yes. I definitely say so. Tried it out afterward and had no problems using it myself. \$\endgroup\$ – michael.zech Mar 21 '16 at 17:09
  • \$\begingroup\$ Ah, I see. An even shorter form of level = level || 0. Awesome. :) Thanks a lot. \$\endgroup\$ – michael.zech Mar 21 '16 at 17:11
  • 1
    \$\begingroup\$ @st88 Not quite. It's a a bitwise OR operation level = level | 0; not a logical OR. It works here because level is intended to be an integer, and bitwise OR zero is "safe". If level was 2 and you used 1 as the "default" instead of zero, you'd get 2 | 1 => 3 \$\endgroup\$ – Flambino Mar 22 '16 at 12:40
5
\$\begingroup\$

Your code could do with some formatting. As it stands, it sure is short, but we have minifiers for that sort of thing. I have to spend a lot of effort to read certain parts because it is filled with clever tricks.

var one = {};
var two = {'one':one};
one.two = two;

If I now pass either of one or two into showObj, it produces an error:

Uncaught RangeError: Maximum call stack size exceeded

Which is worse than using the console, given that console.log(one) would have given me a nice expandable object, in chrome at least. You might want to handle circular references like this. Printing any sort of doubly linked-list will kill your showObj.

\$\endgroup\$
  • \$\begingroup\$ This can be handled by checking if the current key is equal to obj, in case it is an object. \$\endgroup\$ – Ismael Miguel Mar 22 '16 at 16:04
  • \$\begingroup\$ @IsmaelMiguel that would resolve most of the issues, but the sad thing is you'll have to keep track of ALL the objects. Else you can do a stack where the bottom node knows the top one: a->b->c->a. Which any html element will probably already have, with offsetParent skipping several links up the chain and children to drop down one link at a time. \$\endgroup\$ – Pimgd Mar 22 '16 at 16:07
  • \$\begingroup\$ I can see that working. Or, you can just limit it to 20 levels or something. \$\endgroup\$ – Ismael Miguel Mar 22 '16 at 16:26
  • \$\begingroup\$ Thanks a lot for this review. Please look at comments in my edited question. \$\endgroup\$ – cFreed Mar 23 '16 at 3:31
  • \$\begingroup\$ Thanks to your comment, I could add the necessary to work with recursive objects. You can look at the improved version here: codereview.stackexchange.com/a/123620/69690. \$\endgroup\$ – cFreed Mar 26 '16 at 1:13
4
\$\begingroup\$

The code is hard to read, in my opinion.

You have stuff everywhere: Same line, next line, mixed up...

Now, let's review the code:

  • You have a recursive function, which takes the 'html' and the current level.

    Now, what if I call showObj(undefined, undefined, -1);?

    It throws a RangeError: repeat count must be non-negative.

    You could have something like this:

    function showObj(obj, html){
        //your variables that won't change come here
        var tabcount = 4;
        var edges = '{[}]';
    
        var recursive = function(obj, html, level){
            [...]
        };
    
        return recursive(obj, html);
    }
    

    This way, only the stuff that matter is passed and no one will change the level.


    Adding that recursive function there will cost you nearly nothing. The impact of it is so small that you should just neglect it.

    If it wastes 10 picoseconds to call the recursive function, I would be surprised.

  • On the 3rd line (or so), you have this code:

    const tabCount = 4,
    edges = '{[}]';
    

    And now, I need to use a transplier to support IE11... (It is the IE version on Windows 8.1, which is somewhat modern.)

    You are using an ES6 feature here (which isn't bad) which causes a huge implementation problem to older browsers. And it is completelly unnecessary!

    1 line and you caused headaches.

  • A few lines below, you have a variable called tab.

    Nothing wrong, except that it doesn't have tabs! You give it spaces or non-breaking spaces (&nbsp;).

    This should be var tab = html ? '&#9;' : '\t';! There, it's a tab.

  • Please, don't use double-negative. Please! It's so wasteful and redundant. !!html == html will always be true.


    Except when it doesn't...

    Boolean(false) != false. Yeah, boolean objects are ALWAYS true.

    To avoid the ugly edge-case and the horrendous double-negative, just use the very simple single negation.

    Or, you could change the name to plaintext and do like this:

    var tab = !plaintext ? '&#9;' : '\t';
    

    Since null, 0, NaN, undefined and '' are falsy values, negating once will give us true. That means that we don't want plaintext but html.

  • You are handling strings, booleans, numbers, symbols and undefineds on different ways.

    Why don't you just do value + ''? It will work for everything, except strings.

    Those would need a bit different treatment.

  • Your string handling is SO SLOW!!!

    You do src.toString(), which is so redundant! You are running this function for every string. As far as I know, strings are already strings. No need to convert a string to a string.

  • A few lines below you use let. This is so unnecessary. You really don't need that extra scope. Just use var.

  • That default there should throw an exception. How else will the programmer know that the type xyz cannot be handler?

    Replace it with this:

    [...]
    
    default:
        throw new TypeError('Unexpected type ' + (typeof obj));
    

    People will love it so much! Way better than returning the same as a successfull call.

  • The object handling is a total mess. Since the beggining of this review, I've tried to understand what is going on. And for the life in me, I have no idea. The terrible double-space indentation, combined with the total lack of comments, provides the perfect environment for messy, unreadable code.

    Not only that, but it's functionality is obscure. The way it works isn't clear. Don't get me wrong: I usually write minified code and increase it's readability. That is a bad way to do things.


    Why am I bashing on the "double-space indentation"? Because it should be tabs.

    Why use tabs? Well, we all prefer different indentation sizes. A tab can be 2, 4 or 8 spaces wide. You can set at your taste. And the best part? It's only 1 byte! If you use spaces for indentation, you are using n bytes.

    Javascript code is sent over the network, where every single bit counts. 1 single byte can force a new package to be sent, which may slow things down to the receiver.

    Your code is 1146 bytes long. That's a lot for a function. Replacing every 2 spaces with tabs, makes the code 1068 bytes long. That is some serious saving! 78 bytes just because you switched to a tab.

  • Bug: You are returning incorrect results. Yeah, you are.

    From the previous mentioned mess, you return arrays, strings, objects and whatever happens to be there. What happened to returning strings?


    What I mean by this is that none of the following examples returns a string or returns one that makes no sense:

    showObj(new Date()) //Returns a Date() object
    showObj(new Number(5)) //Returns the Number() object
    showObj(new String(5)) //Returns ... some weird stuff...
    showObj(new Object(5)) //Returns a Number() object
    showObj(new Image()) //Returns an <img> element
    showObj(this) //Returns the window object
    

    This shows how brittle the code is. It breaks with anything.

  • Bug: An empty array is different than an array with elements.

    Try this:

    showObj([]) //Array []
    showObj([1]) //"[1]"
    

    Weird...

  • Bug: It doesn't work on Internet Explorer!

    It simply throws an The objeto doesn't support the property or the method 'repeat'.

    And I'm talking about Internet Explorer v11.

    This isn't old: it's the version shipped with Windows 8.1. (I know that there's Windows 10, but don't go that way...)

And with this, I conclude my review. As I said before: the code is hard to read, follow and is very brittle.

Object handling is it's weakness.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for this review. Please look at comments in my edited question. \$\endgroup\$ – cFreed Mar 23 '16 at 3:31
  • \$\begingroup\$ Just now I realize you'd edited your review (didn't get any notification). From your additional comment about html I found I didn't pay attention to my 2 other !! at this point, so I now dropped them as well. Regarding tabs, I think it's kind of opinionated subject, and it can't be discussed here. Nevertheless note that most editors today automatically convert tabs to spaces by default (and notably the SO snippet tool, and also uses 2 spaces indentation). In the other hand, you didn't explain your ununderstood question yet, nor commented about my explanation about what you called "a mess". \$\endgroup\$ – cFreed Mar 26 '16 at 1:32
  • \$\begingroup\$ Oh, I missed your other new comment about strings. Obviously, you're right. It came from a development step where toString() was used commonly for string and other types. And once this changed I didn't pay attention to what could be simplified here. That's done now. \$\endgroup\$ – cFreed Mar 26 '16 at 1:38
  • \$\begingroup\$ @cFreed Why is it a mess? Because it is unreadable, confusing, tangled and (even after 4 days) I still can't read it! I have no idea what's going on there. \$\endgroup\$ – Ismael Miguel Mar 26 '16 at 1:38
  • \$\begingroup\$ Can't you talk quietly, without being disobliging? You seem constantly angry! Regarding your last comment, it doesn't explain anything as is. If you want to be constructive, you must find other words to describe your personal view about what would be cleaner. \$\endgroup\$ – cFreed Mar 26 '16 at 1:44
0
\$\begingroup\$

Here is a modified version of my original code. It has been improved using the reviews I already get (as detailed under the code below):

  • normalize some coding aspects
  • correct the invalid output for an empty array
  • take care of possible recursion

I also added the needed code to compact arrays and objects when they can fit in a single line.

I remain interested to other reviews, TIA.

/**
 * @param src: any variable of any type
 * @param html: output format (true|false); default = false
 * @param level: (internal, don't use)
 * @param history: (internal, don't use)
 *
 * @return string: formatted output
 */
function showObj(src, html, level, history) {
  level |= 0;
  history = history || [];
  const tabCount = 4,
        maxString = 40,
        edges = '{[}]}';
  var sp = (html ? '&nbsp;' : ' '),
      br = html ? '<br>' : '\n',
      tab = '',
      offset = '',
      i;
  for (i = 0; i < tabCount; i++) {
    tab += sp;
  }
  for (i = 0; i < level; i++) {
    offset += tab;
  }
  switch (typeof src) {
    case 'boolean':
    case 'number':
    case 'undefined': return src + '';
    case 'string': return '"' + src.substr(0, maxString) +
      (src.length > maxString ? '...' : '') + '"';
    case 'symbol': return src.toString();
    case 'function': return (src.name || 'function') + '()';
    case 'object':
      if (src === null) {return 'null';}
      if (src === this) {return src.valueOf();}
      if (src.jquery) {return 'jQuery ' + src.jquery + ' object';}
      if (history.indexOf(src) > -1) {return 'recursion';}
      history.push(src);
      var isArr = Array.isArray(src);
      if (!isArr && !Object.keys(src).length) {return src;}
      var output = [];
      for (var key in src) {
        output.push(
          br + offset + tab + (isArr ? '' : ('"' + key + '": ')) +
          showObj(src[key], html, level + 1, history)
        );
      }
      output = edges[+isArr] + output.join(', ') + br + offset + edges[+isArr + 2];
      var line = output.replace(
        new RegExp(br.replace(/\n/, '\\\n') + '(&nbsp;|\\s){' + tabCount + ',}', 'g'),
        '');
      return line.length <= maxString ? line : output;
    default:
      return '(unexpected!) ' + typeof src;
  }
}

Here is it in action:

/**
 * @param src: any variable of any type
 * @param html: output format (true|false); default = false
 * @param level: (internal, don't use)
 * @param history: (internal, don't use)
 *
 * @return string: formatted output
 */
function showObj(src, html, level, history) {
  level |= 0;
  history = history || [];
  const tabCount = 4,
        maxString = 40,
        edges = '{[}]}';
  var sp = (html ? '&nbsp;' : ' '),
      br = html ? '<br>' : '\n',
      tab = '',
      offset = '',
      i;
  for (i=0; i < tabCount; i++) {
    tab += sp;
  }
  for (i=0; i < level; i++) {
    offset += tab;
  }
  switch (typeof src) {
    case 'boolean':
    case 'number':
    case 'undefined': return src + '';
    case 'string': return '"' + src.substr(0, maxString) +
      (src.length > maxString ? '...' : '') + '"';
    case 'symbol': return src.toString();
    case 'function': return (src.name || 'function') + '()';
    case 'object':
      if (src === null) {return 'null';}
      if (src === this) {return src.valueOf();}
      if (src.jquery) {return 'jQuery ' + src.jquery + ' object';}
      if (history.indexOf(src) > -1) {return 'recursion';}
      history.push(src);
      var isArr = Array.isArray(src);
      if (!isArr && !Object.keys(src).length) {return src;}
      var output = [];
      for (var key in src) {
        output.push(
          br + offset + tab + (isArr ? '' : ('"' + key + '": ')) +
          showObj(src[key], html, level + 1, history)
        );
      }
      output = edges[+isArr] + output.join(', ') + br + offset + edges[+isArr + 2];
      var line = output.replace(
        new RegExp(br.replace(/\n/, '\\\n') + '(&nbsp;|\\s){' + tabCount + ',}', 'g'),
        '');
      return line.length <= maxString ? line : output;
    default:
      return '(unexpected!) ' + typeof src;
  }
}

var recursiveObj = {};
var otherObj = {'recursive-obj': recursiveObj};
recursiveObj.otherObj = otherObj;

var someSymbol;
try {
  someSymbol = Symbol('someSymbol');
} catch(e) {
  someSymbol = 'IE unknown: Symbol()'
}

var src = {
  'null': null,
  'undefined': undefined,
  'this': this,
  'integer': 123,
  'float': 12.3,
  'Infinity': Infinity,
  'NaN': NaN,
  'Math.PI': Math.PI,
  'string': 'Some short string',
  'long-string': showObj.toString(),
  'bool-true': true,
  'bool-false': false,
  'date': new Date(),
  'regexp': /.*/i,
  'error': new Error('Error message'),
  'symbol': someSymbol,
  'function': function someFunc() {},
  'anonymous-func': function() {},
  'empty-array': [],
  'simple-array': [1, 2, 3],
  'simple-object': {a: 'A', b: 'B', c: 'C'},
  'compound-object': {
    'compound-array': [
      {'integer': 123, 'float': 12.3},
      {'NaN': NaN, 'string': 'A string'},
      {
        'sub-array': [1, 2, 3],
        'sub-object-short': {a: 'A', b: 'B', c: 'C'},
        'sub-object-long': {a: 'A', b: 'B', c: 'C', d: 'D', e: 'E'},
      },
    ],
    'recursion': recursiveObj,
    'recursion-again': otherObj
  },
  'jQuery': $('body'),
  'map': new Map([['key1', 'value1'], ['key2', 'value2']]),
  'set': new Set(['value1', 'value2', 'value3'])
};

console.log(showObj(src));
document.write(showObj(src, true));
* {
  font-family: "Courier New";
  font-size: 12px;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

Here is the detailed comments about the review I already get.

Common usage of src + '' for several types.

Thanks to @Ismael Miguel: I didn't realize this trick could be used for both boolean, number and undefined types. Though note that it can't be used for Symbol: it fires a ReferenceError: x is not defined.

Usage of let.

For a few time I took the habit of prefering let in a block, as a principle to avoid any possible collision, without having to think more.

From the way @Ismael Miguel wrote his comment I guessed it might have a cost, so I googled and discovered that (even it remains not really clear) it could impact performance. So henceforth I'll take it in account, thanks.

Usage of !!.

Like for the above, it results from an habit I've taken, because one time I faced a special tortuous case where it was the way I found to express a double check with only one statement. I admit it's not a good idea to use it as soon as I want to avoid thinking deeply :)

Case of an empty Array.

@Ismael Miguel, you're right, and I didn't pay attention to that. It's now corrected... but only for Array (see below regarding Object). It's of interest to note that it happened because of this test:

if (!Object.keys(src).length) {return src;}

that is the only way I found to detect objects like Map and Set, and so return at least an information about what they are, rather than merely outputting them as {}. I'd be interested to get suggestions about how to do this better.

Note that, at the opposite, the issue happens for a purely empty Object: showObj({}) outputs [object Object], while it'd better to get {}. Here also I'm interested by your suggestions.

Case of recursion.

Many thanks to @Pimgd for pointing that: it's "the" big point I'd totally missed!

This lack is now corrected.

About some of my deliberate choices.

First of all, please remember the introducing text of my question, which stated that this snippet is intended to be a tool I'll use in SO answers and nothing else.

Hence I clearly don't want to make it heavier than strictly needed in this context. So there are several comments with which I would totally agree if this code was intended to serve in an application but will not hold to this case:

  • Prevent against outer call to showObj() providing the level argument.
  • Make it compliant with older browsers.
  • Throw an exception in case of unexpected variable type (especially here: we don't want exceptions to fire, we expect to get information about the analyzed variable).

A peculiar point regards my tab variable name: that's totally wanted! For times we read and/or heard that using real \t chars was the best way when editing code. I always disagreed: it leads to inconsistent results when manipulating code from different source because of the different configurations (tab counts) of the editors. Fortunately it's now admitted that, at the opposite, the best way is to only use spaces, and it's what I'm doing here, but keeping the name tab to reflect what is it's actual usage.

About readability.

Note that when I said I "tried to make it clear but concise", it addressed the output result, not the code (but reading my text again now I admit it was not fully clear).

In the other hand, regarding the code, I essentially tried to make it occupy only a reasonable height, as long as it involved very simple statements. This is why several times I wrote something like:

case 'number': return src.valueOf();
case 'string': return '"' + src.toString() + '"';

rather than:

case 'number':
  return src.valueOf();
case 'string':
  return '"' + src.toString() + '"';

and so on...

BTW I'm quite surprised it may be found unreadable. For me, at the opposite, in such a case it's something like an optimization, in the sense of the word in the DB realm: one you've designed your structure following strict rules, you deliberately trangress some of them to gain efficiency.

I'm even more surprised it may be found messy! Let's examine how it's ordered, very merely:

  • First appear all "simple" types of data, then other ones.
  • Inside each of those two categories, the different types appear alphabetically.

At the beginning I considered to be guided by technical aspects (like "this or that type must be processed first"), but actually didn't faced any such constraint, hence the sorting order I retained . Feel free to suggest me another one if it can bring an advantage.

Another aspect regards the absence of comments. A lot of SO answers condemn lot of questions where the code include useless comments, because they don't bring any additional information. And here is a good example, IMO, of code where I can't see the opportunity to add a comment which wouldn't be redundant with the code itself.

A comment I don't understand.

I can't figure out what @Ismael Miguel means with this:

From the previous mentioned mess, you return arrays, strings, objects and whatever happens to be there. What happened to returning strings?

TIA to clarify!

\$\endgroup\$
  • 4
    \$\begingroup\$ If you are still interested in more reviews, I'd recommend posting a new question. It is very common around here to post "follow-up" questions. \$\endgroup\$ – Simon Forsberg Mar 23 '16 at 18:39
  • 2
    \$\begingroup\$ Another option is to discuss things in chat. Chat rooms doesn't necessarily need to be instant, you are free to reply to chat messages whenever you have time, there's nothing forcing you to reply instantly. \$\endgroup\$ – Simon Forsberg Mar 23 '16 at 18:41
  • \$\begingroup\$ @SimonForsberg Thanks for your suggestion in your first comment above, but I'm embarrassed. The link where Quill addressed me (codereview.stackexchange.com/help/someone-answers) recommends posting either a new question or a self-answer: then I chose the 2nd solution. It seemed more consistent, because my new post includes a lot of comments directly related to the reviews I got _here. \$\endgroup\$ – cFreed Mar 23 '16 at 19:40
  • \$\begingroup\$ @SimonForsberg Regarding your second comment, I suppose it relates to my exchange with Ismael Miguel. I must confess I'm not very tempted: I upvoted his review and posted detailed (and temperate) reflexions about it, then he merely respond that I don't understand anything, and other so kind words! With that, added to my poor English, I don't think I could argue quietly and constructively in chat... \$\endgroup\$ – cFreed Mar 23 '16 at 19:47
-2
\$\begingroup\$

With your description and documentation I could read the code from top to bottom. Understood at once which way things are accomplished.

Therefore: Clear? Yes. I definitely say so.

Tried it out afterward and had no problems using it myself.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the kind words. Nevertheless, I think that it's not in the CR intents that users post answers without any critical comment. I guess it's probably why somebody downvoted yours, but I find it a bit too harsh! So I upvote to compensate. \$\endgroup\$ – cFreed Mar 22 '16 at 15:24
  • 3
    \$\begingroup\$ @cFreed Yes on Code Review most answers will find something to suggest an improvement on. Answers that review the code but conclude that it seems good are rare, but perfectly valid. \$\endgroup\$ – SuperBiasedMan Mar 22 '16 at 15:28
  • 4
    \$\begingroup\$ While this is an answer, it is a bit strange that you also wrote as a comment: "What's the meaning of the |= operator in "level |= 0" ? I have never seen that before." \$\endgroup\$ – Simon Forsberg Mar 23 '16 at 19:02

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.