Ad
  • Custom User Avatar

    No fixed tests where the node's value is equal to its parent node's value. Random tests generate such cases way too rarely as well.

  • Custom User Avatar
  • Custom User Avatar
  • Custom User Avatar

    Fixed. Also, updated to Node 14.x.

  • Custom User Avatar

    Just a minor clarification. if anyone is stuck, that's probably because you should not recurse into child nodes when the child is less than its parent.

    I am not marking this as having spoiler content since this guideline is included in the description, albeit hidden inside the code block, titled "Example of a tree".

  • Custom User Avatar
    • As mentioned below, please use Test.assertEquals and not Test.expect
    • Needs random tests
  • Custom User Avatar

    Is it really an issue? ;-)

  • Custom User Avatar

    You can show the visual tree of your example, it's a 'nice to have'

  • Custom User Avatar

    I had more trouble interpreting the description that necessary. The: "counts how many nodes can be traversed from root without any child node having smaller value than its parent."

    can be interpreted as
    "find the longest non-decreasing chain from the root"

    where the author actually means
    "count the number of non-decreasing nodes reachable from the root".

  • Custom User Avatar

    The way to use Test.randomize is to create an array of test data, randomize it, then run the tests over the randomized list.

    Something like this:

    // inside your describe/it block
    
    // Randomize some nodes
    var tests = Test.randomize([
       // Tree, Expected Nodes, Msg
       //   (You could also skip the message, and build it dynamically
       //    in the function below.)
       [{...}, 34, 'There should be 34 nodes in this tree'],
       [{...}, 12, 'There should be 12 nodes in this tree'],
       ...
    ]);
    
    tests.forEach(function(data) {
      Test.assertEquals(countNodes(data[0]), data[1], data[2]);
    });
    

    Now, I wouldn't randomize every test, just a few at the end. Otherwise it becomes hard to solve, because you won't get the same results.


    Another thing that would be a good habit is to use more meaningful messages — either within the Test.assertEquals() itself, or within it messages. For example, your setup could look like this:

    describe("Non-decreasing nodes", function() {
      it('should handle single a node tree', function() {
        Test.assertEquals(countNodes({value: 1, left: null, right: null}), 1, 'There should be 1 non-decreasing node in this tree');
      });
      
      it('should handle a left-node-only tree', function() {
        Test.assertEquals(countNodes({value: -1, left: {value: 2}}), 2, 'There should be 1 non-decreasing node in this tree');
      });
      
      it('should handle a null value (empty tree)', function() {
        Test.assertEquals(countNodes(null), 0, 'There should be 0 non-decreasing nodes in this tree');
      });
    });
    
    describe("Randomized Input", function() {
      // put the random block here
    });
    

    See how the descriptions explain what is being tested, rather than just the expected output? This helps immensely when a test fails. Now you are giving organized, categorized feedback on the results of the test.

    Note: You can have multiple tests within an it block, but usually these are testing the same input data. Since you only have a single integer as output, there's not going to be multiple tests. (And example where you might have multiple tests is complex output such as Objects or Arrays, or testing how many times a function is being called.)

  • Custom User Avatar

    Thanks for the detailed advice! I hope it looks better now, although I'm not sure if randomize is working as it should...

  • Custom User Avatar

    This is a nice idea for a kata. A few things that would improve it:

    • You need a little more detail in your spec. It's solveable as is, but more exact specifications could help with optmizing the solution.

      • Are the numbers always integers?
      • Are negative numbers and/or zero going to be presented?
      • The example is nice, but some users may prefer a textual spec rather than assuming one from the code. In the real world, it would definitely be good to spell out the structure of your tree explicitly, rather than implicitly. This isn't critical here, just a good habit.
      • Will the function always be passed a valid tree? (e.g., will null be passed in?)
      • If the input may be invalid, what's the correct behavior?
    • Replace Test.expect(test) with Test.assertEquals(actual, expected, msg), like so:

      Test.assertEquals(countNodes(/* tree */), 42, 'There should be 42 nodes in this tree');
      

      This is better on 2 counts:

      1. Test.assertEquals() will render both the expected value and the actual value, which helps in debugging, without immediately resorting to console.log.
      2. Providing a message can be useful, although I'm not sure it's necessary here. Since the output is a single number, and you are probably not going to render the tree for the user.
    • Adding in a few more test cases might be good

    • If you do this, also wrap your test cases in describe('category', fn) and it('test description', fn) blocks. The docs have examples on how to use these. These blocks make your test code cleaner and more structured.

    • Add some randomization to your test cases.

      Right now, it would be trivial to "cheat" by returning the correct answers. You can use Test.randomize(array) to randomly order an array, then test each item in that array separately. This is usually good enough to thwart "cheating". I usually prefer to have a separate set of simple, random cases just for this purpose.