binary-search-tree: implement canonical-data.json #940

devkabiir · 2017-10-10T05:53:05Z

Closes #555

devkabiir · 2017-10-10T05:55:12Z

Since I don't know who to ask for review @kytrinyx I'll leave that to you.

petertseng · 2017-10-10T05:58:08Z

exercises/binary-search-tree/canonical-data.json

+        "null should be replaced by the respective language equivalent or -1",
+        "if the language does not have any.",
+
+        "The purpose of ths exercise is to create a data structure which",


I'm unclear on how this sentence affects someone implementing this exercise for a given track, and I don't think there was any danger of anyone using a sorting function for this exercise anyway? Will you explain?

Well as i understand this exercise from description.md the purpose is to create a data structure which represent sorted data and specifically binary search tree. Usage of any sort() function or algorithm will defeat its purpose don't you think?

Usage of any sort() function or algorithm will defeat its purpose don't you think?

Agree.

I don't think it is likely that anyone will think to use a sorting function, because they should already be at the point where they're inserting nodes into a tree.

Therefore, I don't think it is useful to have this text here, and I will find it confusing becuase I will read it and think "why are you telling me this?"

It could be that I am biased and this text does help those who see it from a different point of view.

It's possible that in future exercism adds a track in which the obvious choice would be to use the sort() function or a sorting algorithm. In this case this exercise might become invalid for that track.

It is also possible that the person contributing the exercise is new and binary search tree is a new concept to them.

Therefore current implementations of the testsuite do test for usually internal/private functions of such a datastructure.

They ask for the left or right subtree of the root node and assert if its value is correct given the insertion order.

The do roughly the following:

root = empty |> insert(1) |> insert(3) |> insert(2) three = left_of(root) two = root |> left_of |> right_of assert value_of(root) = 1 assert value_of(three) = 3 assert value_of(two= 2

So not only testing if the tree contains the values, but also if it is built correctly is not possible by just using a sort function, since original order of insertion dictates the structure of the tree.

So I do not think that bullet 1 is a concern if you are testing for internals in this canonical data. If you do not, please add those tests.

A person that is new to the BST, will see the README.md which explains the exercise to solve. The comments in the canonical data are meant as a guidance/help for the implementor of a testsuite/a generator, not to describe the actual exercise.

Because of this two reasons, I am with @petertseng and think this part of the comment should be removed,

petertseng · 2017-10-10T06:00:13Z

exercises/binary-search-tree/canonical-data.json

+    "cases": [{
+            "description": "insert invalid/null data into the tree",
+            "property": "insert",
+            "value": null,


Similar reasoning as #925: I don't think it is good to have this case.

In most languages that have static type checking, even if they have nullable references, integers are not reference types, so if the function under test is defined to take an integer, null is not a possible input.

For languages without static type checking, null would be a possible input, but I also generally do not see functions that expect to take integers explicitly check for null (or language equivalent); let me know if I'm wrong. So I don't think the tests need to make students write it.

As I stated in the comments : if the specific track does not have any equivalent to null they should use -1. The purpose of this case is not limited to null but actually invalid data. I knew about #925 when I was writing this case and that's why I added the above comment.

if the specific track does not have any equivalent to null they should use -1. The purpose of this case is not limited to null but actually invalid data

I don't understand why -1 is invalid for any reason other than "the comments of this file say so".

I think it is a strange restriction to only allow positive numbers. It seems like the restriction is there for the sole purpose of classifying some numbers to be invalid, allowing there to be a test for inserting invalid values. If that is the sole purpose of the restriction, I would think it just makes more sense to not have the restriction and not have this test.

Further, if negative values are to be deemed invalid values for the tree being tested, that seems like information that is very important to be added in description.md, where students will see it, not canonical-data.json (where students will not see it!).

Well the exercise is supposed to teach how to implement a data structure. And a data structure should know the difference between valid and invalid data before inserting it. Now some languages do not have static type checking whereas some don't have equivalent of null. But we still need to draw a line between the two. And I agree the description.md might need an update.

I think this case should exist, because if we draw a line between valid and invalid data and specify in the description we should also have a test case to check against it.

And a data structure should know the difference between valid and invalid data before inserting it.

I expect that for a BST, by contract, the criterion for validity is a standard "are two elements comparable against each other?". I therefore also expect that this is the criterion that the tests will use, rather than a different standard ("is the number positive?", etc.)

So then, in a language with static type checking, this would get enforced by the type system. In a language without one, now it gets interesting. We could consider a test of adding in "hello" to a tree that already contains a 6, for example. This would fail. However, is it the student's responsibility to write the code that checks for that? The failure might happen without the student having to write any additional code. If it would, then there is no value in having this test.

the exercise is supposed to teach how to implement a data structure.

This makes it useful to keep the standard criterion for validity. If the exercise adds in an additional criterion, students may wonder why it exists and/or whether they have to add such an additional restriction in any future BSTs they write.

So I guess the case for invalid data should exist but anything other than an integer should be considered as invalid. Right?

anything other than an integer should be considered as invalid

Right.

the case for invalid data should exist

Only if it is useful. I haven't been convinced it is useful because in most cases the student doesn't need to write any code to pass the test. Okay, I can name at least one language where 6 < "hello" is not a type error (whether that be at run time or compile time), so in that language it could be useful.

I can't tell whether #902 is telling us "yes, put such cases in the canonical-data.json and let languages that don't need the test ignore it" or "no, don't put such cases in and let languages that need the tests add them". I am thinking the number of languages that benefit from the case is lower than the number of languages that don't, if that helps decide.

I still say no. Remember that it's not necessary to convince me, it's only necessary to convince those who approve the pull request.

A BST is about comperability, and I do not see why string and integer should not be comparable to each other...

In the erlang VM (and therefore in at least 3 languages on exercism) there is a total order defined over all values possible. 6 =< "foo" is true in erlang, lfe and elixir (in their corresponding syntaxes).

Also after having read the introductionary comment, where you state that we only use positiv numbers as valid input, I'd use an unsigned int in statically typed languages.

But you have specified -1 as an error indicator, but the only thing that can cause an error, is invalid input (which can't happen in accordingly typed languages) or someone else mangles with the tree from the outside, which we suppose not to happen.

So independently how I look at it, I do not feel that this test is necessary in any way.

petertseng · 2017-10-10T06:00:24Z

exercises/binary-search-tree/canonical-data.json

+        {
+            "description": "search invalid/null data",
+            "property": "search",
+            "value": null,


Similar reasoning as #925: I don't think it is good to have this case.

In most languages that have static type checking, even if they have nullable references, integers are not reference types, so if the function under test is defined to take an integer, null is not a possible input.

For languages without static type checking, null would be a possible input, but I also generally do not see functions that expect to take integers explicitly check for null (or language equivalent); let me know if I'm wrong. So I don't think the tests need to make students write it.

petertseng · 2017-10-10T06:01:57Z

exercises/binary-search-tree/canonical-data.json

+        {
+            "description": "insert a number that already exist",
+            "property": "insert",
+            "value": 6,


I didn't see how we can understand that the number already exists, using the information provided in this case. If we suppose that we are inserting into a tree that already had 6 inserted into it at some point, that needs to be specified in some way here. Otherwise, compare this case with the case immediately above it: the only difference is the description and expected values, so there doesn't seem to be any apparent reason why one would expect true and the other would expect false.

petertseng · 2017-10-10T06:02:41Z

exercises/binary-search-tree/canonical-data.json

+            "description": "search a number that exists",
+            "property": "search",
+            "value": 6,
+            "expected": true


How do we know that 6 exists in the tree? What tree is this search being performed on?

petertseng · 2017-10-10T06:02:56Z

exercises/binary-search-tree/canonical-data.json

+            "description": "search a number that does not exist",
+            "property": "search",
+            "value": 11,
+            "expected": false


How do we know that 11 does not exist in the tree? What tree is this search being performed on?

petertseng · 2017-10-10T06:04:04Z

exercises/binary-search-tree/canonical-data.json

+            "property": "ascendingOrder",
+            "comments": [
+                "Assuming the data is entered in this order :",
+                "6 -> 1 -> 3 -> 2 -> 5"


it looks like these values should be an array of numbers, rather than forcing people to parse this string, splitting by -> to find the numbers, etc

I thought about it but i wasn't sure if i should add a new key called "state" which represents the data entered in the tree at that case in that specific order. This new key "state" will also prove useful in previous cases where its not clear what data the tree currently has.

I think there has to be such a key. You could call it previously_inserted_values instead of state if needing a more descriptive name.

Um. maybe current_tree_data because then i can also have this key in all cases and make all cases consistent.

I'm not even sure how to read this...

6, then 1, then 3, then...

or is it

6 after, 1 after 3, after...

or

6 has child 1, has child 3, has child (but which child then?)

Some implementations ask for implementing a function that creates a BST from a list, growing the tree from the first item to the last.

Roughly like this:

foldl Leaf (\ x t -> insert t x) list

In the introductionary comment we could explain that a property "value" could be used to create a singleton tree, or inserting a single value to a given tree, while "values" or "tree" implies to create a tree from a list using this function.

petertseng · 2017-10-10T06:04:09Z

exercises/binary-search-tree/canonical-data.json

+            "property": "descendingOrder",
+            "comments": [
+                "Assuming the data is entered in this order :",
+                "6 -> 1 -> 3 -> 2 -> 5"


it looks like these values should be an array of numbers, rather than forcing people to parse this string, splitting by -> to find the numbers, etc

devkabiir · 2017-10-15T07:38:35Z

So in conclusion I should

remove the invalid data` test
add a current_tree_data key to every test which is an array.
add a comment which explains the order of the data in curernt_tree_data
e.g. [6,5,3,1] means insert 6 then insert 5 then insert 3 then insert 1.

I'd like your opinions on the order of tests too and if I should add another test. @petertseng @NobbZ

It's been a busy week for me, hence the late reply.

petertseng · 2017-10-15T22:41:51Z

So in conclusion I should

remove the invalid data` test

add a current_tree_data key to every test which is an array.

add a comment which explains the order of the data in curernt_tree_data
e.g. [6,5,3,1] means insert 6 then insert 5 then insert 3 then insert 1.

I agree with that

I'd like your opinions on the order of tests too and if I should add another test

I defer to all tracks that have implemented this exercise.

$ curl http://x.exercism.io/v3/problems/data-todos | jq '.[] | map(select(.["slug"] == "binary-search-tree"))'

[
  {
    "slug": "binary-search-tree",
    "blurb": "Insert and search for numbers in a binary tree.",
    "readme_url": "https://github.com/exercism/problem-specifications/blob/master/exercises/binary-search-tree/description.md",
    "data": null,
    "implementations": [
      {
        "track_id": "csharp",
        "url": "https://github.com/exercism/csharp/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "clojure",
        "url": "https://github.com/exercism/clojure/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "coffeescript",
        "url": "https://github.com/exercism/coffeescript/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "crystal",
        "url": "https://github.com/exercism/crystal/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "ecmascript",
        "url": "https://github.com/exercism/ecmascript/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "elixir",
        "url": "https://github.com/exercism/elixir/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "fsharp",
        "url": "https://github.com/exercism/fsharp/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "go",
        "url": "https://github.com/exercism/go/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "haskell",
        "url": "https://github.com/exercism/haskell/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "java",
        "url": "https://github.com/exercism/java/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "javascript",
        "url": "https://github.com/exercism/javascript/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "lua",
        "url": "https://github.com/exercism/lua/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "perl5",
        "url": "https://github.com/exercism/perl5/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "ruby",
        "url": "https://github.com/exercism/ruby/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "scala",
        "url": "https://github.com/exercism/scala/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "swift",
        "url": "https://github.com/exercism/swift/tree/master/exercises/binary-search-tree"
      },
      {
        "track_id": "typescript",
        "url": "https://github.com/exercism/typescript/tree/master/exercises/binary-search-tree"
      }
    ]
  }
]

The suggestions in https://github.com/exercism/docs/blob/master/you-can-help/improve-exercise-metadata.md#extracting-canonical-test-data say to consult the implementations to see what tests to add. I am presently unable to do this on your behalf, so I trust you to do it.

Insti · 2017-10-31T09:17:28Z

@kabiir are you still working on this?

devkabiir · 2017-11-02T01:01:35Z

@Insti Yes!, I just didn't get enough time. I've been going through tracks that have implemented tests, they all seem to follow a trend it's like there is a hidden canonical-data.json. Anyway i'm working on it and i'll try to follow the trend too.

ErikSchierboom · 2017-11-02T07:40:52Z

they all seem to follow a trend it's like there is a hidden canonical-data.json

Which is probably entirely due to the fact that people often copy an existing set of tests and modify them to fit their track :) Great to hear that you are working on this by the way!

devkabiir · 2017-11-02T23:56:00Z

So for test case "insert smaller number on left node" and data [4, 2], to check
tree.root.left.data = 2 which means left node of root contains 2. How should I represent this test case?
More importantly how should I represent root.left node. Should I create a new key called node: "root.left" and set property:"data"
e.g.

               {
                    "description": "smaller number at left node",
                    "property": "data",
                    "node": "root.left",
                    "tree_data": [4, 2],
                    "expected": 2
                }

Or maybe I could add a comments key and explain in plain english for every test case
e.g.

               {
                    "description": "smaller number at left node",
                    "property": "data",
                    "comments": "Left child of node '4' should be node '2'",
                    "tree_data": [4, 2],
                    "expected": 2
                }

I'm inclined to use comments. Thoughts? @petertseng @NobbZ @Insti

NobbZ · 2017-11-03T06:29:57Z

My generator were not able to parse those comments. Therefore I think we need to find a way to do it in a formalized way, but I'M undercaffeinated, so I do not have a clue how right now…

petertseng · 2017-11-03T22:31:04Z

You may use a series of operations as in https://github.com/exercism/problem-specifications/blob/master/exercises/zipper/canonical-data.json#L86-L96 if you desire.

This statement must not be taken as an endorsement or rejection of using a series of operations.

devkabiir · 2017-11-03T23:53:20Z

Since most tests are expecting the tree to be in a certain form. Adding an expectedTree key which represents the tree, will the following representation do?

"comments": [
        "The key expectedTree represents a tree as a node object starting from root node",
        "a node object consists of 3 keys 'data', 'left' and 'right'",
        "The key 'data' contains data of the said node, 'left' contains",
        "a node object of left-child node and 'right' contains a node object",
        "of right-child node."
]
{
    "description": "greater number at right node",
    "property": "insert",
    "tree_data": [4, 5],
    "expectedTree": {
        "data": 4,
        "left": null,
        "right": {
            "data": 5,
            "left": null,
            "right": null
        }
    }
}

petertseng · 2017-11-04T22:05:10Z

I agree that our goal should be to check that the tree resulting from inserting a given sequence of numbers has a shape matching the expected shape.

I advise that if the trees are implemented as abstract data types that an outside caller such as a test suite may not be able to directly construct a tree that can be compared against the observed tree (otherwise it would force the implementation), so they may be restricted to using the interface to test the shape of the tree.

This advice doesn't prevent the proposed representation, therefore it is not a rejection of the proposal.

The schema will require that expectedTree instead be expected.

coriolinus · 2017-11-05T01:24:49Z

One advantage of the "node": "root.left" proposal is that it succinctly describes the traversal path. For certain tree implementations and languages, the test runner may be able to literally eval the node item and compare it to the expected value. It's a less verbose variant of the sequence of operations idea.

A disadvantage is that it doesn't represent the entire tree, which increases the number of tests required to determine that the whole tree is in the state expected.

I'm moderately in favor of the traversal path suggestion even so, because I suspect that for most tests, we don't need to test the entire tree state to determine whether or not the student has done the right thing; we just need to ensure that the result of the tested operation is as expected.

coriolinus

Generally looks very good!

Per #996, we're modifying policy such that inputs should all appear under the input key. For new canonical data, I think it best to make the change now. For this exercise, that should be a simple matter of replacing all instances of tree_data with input.

This helps test generators keep other data, such as the node property, out of the test inputs.

Insti · 2017-11-21T08:36:12Z

that should be a simple matter of replacing all instances of tree_data with input.

You'll need to wrap it rather than just replace it.

 "input": ["4", "2"],

Would still not match the specification.
It will need to be:

"input": { "tree_data": ["4", "2"] },

devkabiir · 2017-11-22T15:24:50Z

@Insti I agree, but I still don't understand how this particular canonical-data can be parsed by test generators and produce a usable output. tree_data is not actually an input! It's just a convenient way of saying
I want the tree to have these values inserted in this order before checking for this test
I used property data because I didn't know how to convey my test case otherwise. I would appreciate some input on this.

coriolinus · 2017-11-22T16:59:21Z

@kabiir I've got exercise generators on my mind because I recently wrote the one the Rust track uses. I can't speak for other tracks' exercise generators, but what the Rust track's generator will do is group the tests by the tested property. For this set of canonical data, there are two tested properties: data and sortedData. For each tested property, it builds a test stub, like this:

fn process_data_case<I, O>(input: I, expected: O) {
    unimplemented!()
}

fn process_sorteddata_case<I, O>(input: I, expected: O) {
    unimplemented!()
}

It then generates a bunch of actual tests written in terms of those functions. Crucially, if the case in the canonical data includes input, it uses the actual input value. If it doesn't, it collects all the properties, excludes the ones it knows about, sorts them by key name, and sends the batch as a tuple to the test_sorteddata_property.

So, in cases such as book-store which don't have an input property, extraneous data may be included. Those cases include a targetgrouping property which is intended as an output hint, but the test generator can't know that. Instead, it is duly passed into the process_total_case function and then ignored forever.

Back to binary-search-tree, if input is a literal list, then type I in both process_*_case functions will be Vec<&'static str>; if it's an object literal, type I will be HashMap<&'static str, Vec<&'static str>>¹. It's the responsibility of the person running the exercise generator to read the comments and description and implement the process_*_case functions appropriately. Assuming we stick with the standard and use object literals, it'll look something like this:

// assuming BST is the name of the student's struct
fn process_data_case(input: HashMap<&'static str, Vec<&'static str>>, expected: HashMap<&'static str, Option<&'static str>>) {
    let mut student_bst = BST::new();
    for item in input.get("tree_data").unwrap() {
        student_bst.insert(item.parse());
    }
    unimplemented!() // TODO: build the expected tree here, which might be kind of tedious
    assert_eq!(student_bst, expected_bst);
}

The exercise generator does the busywork of generating a whole bunch of tests which look like this²:

#[test]
#[ignore]
fn test_data_is_retained() {
    process_data_case(hashmap!("tree_data" => vec!["4"]), hashmap!("data" => "4", "left" => None, "right" => None));
}

However, it's not the responsibility of the exercise generator to get a complete exercise ready to merge without human input. It's just there to streamline and simplify the work of generating an exercise.

Why are you passing these numbers as strings, by the way? Numeric literals would make perfect sense here. It's not a blocker for me because strings also compare, but it seems odd.
If you're fluent in Rust, you'll notice that the generated tests won't actually compile at all in this case due to a type error in the expected hashmaps. This is a known limitation of the test generator; it can't currently tell when it should assume things are actually hashmaps, and when they're heterogenous objects. I'm working on it, but as a proper solution would likely involve full type inference, a proper solution is likely to be some time away.

Insti · 2017-11-22T18:19:34Z

tree_data is not actually an input!

It is a precondition to the test thus it counts as input to the test generator even if it's not going to be an argument to the function being tested.

devkabiir · 2017-11-23T00:58:05Z

@coriolinus I saw that some of the tracks were using strings to represent numbers so I made the choice of using strings because the person who will implement the test whether using test generators or not will know that data is not restricted to integers only. Since depending upon the test the student will model there program and its not described in the Readme which data type input to BST is going to be. So the tests should not restrict it to numbers or strings but I had to make a choice so strings.

So I get it that the canonical-data doesn't necessarily have to be 100% test generators friendly right? I mean I can try to follow all of the standard but at some point some test cases just won't be parsed by test generators

@Insti Should I mention that on the comment?

Insti · 2017-11-23T09:27:11Z

I can try to follow all of the standard but at some point some test cases just won't be parsed by test generators

Why do you think this would be the case?
Test generators are customised to the particular exercise they are generating tests for.
Just ensure that the format is consistent and logical and there won't be a problem.

Should I mention that on the comment?

Probably.

devkabiir · 2017-11-25T10:30:04Z

@Insti

Why do you think this would be the case?
Test generators are customised to the particular exercise they are generating tests for.
Just ensure that the format is consistent and logical and there won't be a problem.

I thought test generators were general purpose for all exercises

petertseng · 2017-11-26T12:19:08Z

exercises/binary-search-tree/canonical-data.json

+                {
+                    "description": "same number at left node",
+                    "property": "data",
+                    "node": "root->left",


I think in this file you have taken your suggestion of #940 (comment) to use expected to represent the entire tree, and not your suggestion of #940 (comment) to list one specific node being traversed to. If you confirm this is true, I think this node line should be removed.

Yes!, good catch.

devkabiir · 2017-12-06T14:30:15Z

exercises/binary-search-tree/canonical-data.json

+            "description": "data is retained",
+            "property": "data",
+            "input": {
+                "tree_data": ["4"]


@coriolinus @Insti
I just saw issue #987
Should the input key tree_data be changed to lowerCamelCase?

From what I understand #998 is not allowing additional properties. so does #987 make sense here?

I have a mild preference for snake_case over lowerCamelCase, but TBH I didn't even think about the applicability of #987 here.

As I understand it, the driving intent behind that issue was to avoid using keys which require special handling because they contain spaces or otherwise are not permissible as variable names. As that's handled by either naming convention, I don't have a preference one way or the other about whether you should update your cases here.

ErikSchierboom · 2018-01-04T07:48:47Z

@rpottsoh @petertseng Is this PR ready to be merged?

rpottsoh

#987 should be followed, otherwise PR looks fine.

rpottsoh · 2018-01-04T13:51:03Z

exercises/binary-search-tree/canonical-data.json

+                    "description": "smaller number at left node",
+                    "property": "data",
+                    "input": {
+                        "tree_data": ["4", "2"]


I think this should be lowerCamelCase in accordance with #987.

rpottsoh · 2018-01-04T13:51:16Z

exercises/binary-search-tree/canonical-data.json

+                    "description": "same number at left node",
+                    "property": "data",
+                    "input": {
+                        "tree_data": ["4", "4"]


I think this should be lowerCamelCase in accordance with #987.

rpottsoh · 2018-01-04T13:51:28Z

exercises/binary-search-tree/canonical-data.json

+                    "description": "greater number at right node",
+                    "property": "data",
+                    "input": {
+                        "tree_data": ["4", "5"]


I think this should be lowerCamelCase in accordance with #987.

rpottsoh · 2018-01-04T13:51:44Z

exercises/binary-search-tree/canonical-data.json

+            "description": "can create complex tree",
+            "property": "data",
+            "input": {
+                "tree_data": ["4", "2", "6", "1", "3", "5", "7"]


I think this should be lowerCamelCase in accordance with #987.

rpottsoh · 2018-01-04T13:52:12Z

exercises/binary-search-tree/canonical-data.json

+                    "description": "can sort single number",
+                    "property": "sortedData",
+                    "input": {
+                        "tree_data": ["2"]


I think this should be lowerCamelCase in accordance with #987.

rpottsoh · 2018-01-04T13:52:22Z

exercises/binary-search-tree/canonical-data.json

+                    "description": "can sort if second number is smaller than first",
+                    "property": "sortedData",
+                    "input": {
+                        "tree_data": ["2", "1"]


I think this should be lowerCamelCase in accordance with #987.

rpottsoh · 2018-01-04T13:52:30Z

exercises/binary-search-tree/canonical-data.json

+                    "description": "can sort if second number is same as first",
+                    "property": "sortedData",
+                    "input": {
+                        "tree_data": ["2", "2"]


I think this should be lowerCamelCase in accordance with #987.

rpottsoh · 2018-01-04T13:52:38Z

exercises/binary-search-tree/canonical-data.json

+                    "description": "can sort if second number is greater than first",
+                    "property": "sortedData",
+                    "input": {
+                        "tree_data": ["2", "3"]


I think this should be lowerCamelCase in accordance with #987.

rpottsoh · 2018-01-04T13:52:47Z

exercises/binary-search-tree/canonical-data.json

+                    "description": "can sort complex tree",
+                    "property": "sortedData",
+                    "input": {
+                        "tree_data": ["2", "1", "3", "6", "7", "5"]


I think this should be lowerCamelCase in accordance with #987.

convert to lowerCamelCase

rpottsoh · 2018-01-05T02:49:40Z

I think this is now ready to be merged. Not sure if it should be squashed.

ErikSchierboom · 2018-01-05T07:16:32Z

I've decided to go for squashing. Thanks @kabiir @rpottsoh ! 🎉

exercism/problem-specifications#940

Stargator · 2018-01-23T05:28:29Z

Way to go @kabiir! That was a lot of work over a period of time.

Canonical version 1.0.0 from exercism/problem-specifications#940 This commit does not contain a test generator for the exercise.

binary-search-tree: implement canonical-data.json

0c589fa

petertseng reviewed Oct 10, 2017

View reviewed changes

devkabiir self-assigned this Oct 10, 2017

cmccandless mentioned this pull request Nov 2, 2017

Implement exercise binary-search-tree exercism/python#773

Merged

add proposed changes

86dd64e

coriolinus reviewed Nov 20, 2017

View reviewed changes

update input schema as per exercism#996

7c13fb8

petertseng reviewed Nov 26, 2017

View reviewed changes

remove ambiguous key

5d23849

devkabiir requested review from NobbZ and petertseng December 6, 2017 13:30

devkabiir requested a review from coriolinus December 6, 2017 13:30

coriolinus approved these changes Dec 6, 2017

View reviewed changes

devkabiir commented Dec 6, 2017

View reviewed changes

petertseng removed their request for review December 6, 2017 17:21

rpottsoh approved these changes Dec 15, 2017

View reviewed changes

rpottsoh requested changes Jan 4, 2018

View reviewed changes

binary-search-tree: "tree_data" -> "treeData"

0ea99f7

convert to lowerCamelCase

rpottsoh approved these changes Jan 5, 2018

View reviewed changes

ErikSchierboom merged commit d5a4db7 into exercism:master Jan 5, 2018

petertseng mentioned this pull request Jan 5, 2018

binary-search-tree 1.0.0.3: Add toList of equal element exercism/haskell#636

Merged

petertseng added a commit to exercism/haskell that referenced this pull request Jan 5, 2018

binary-search-tree 1.0.0.3: Add toList of equal element (#636)

ca07a7a

exercism/problem-specifications#940

sshine pushed a commit to exercism/ocaml that referenced this pull request Oct 17, 2019

binary-search-tree: port exercise (#370)

7cbe850

Canonical version 1.0.0 from exercism/problem-specifications#940 This commit does not contain a test generator for the exercise.

binary-search-tree: implement canonical-data.json #940

binary-search-tree: implement canonical-data.json #940

Conversation

devkabiir commented Oct 10, 2017 • edited by Insti Loading

devkabiir commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petertseng Oct 10, 2017 • edited Loading

Choose a reason for hiding this comment

devkabiir Oct 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devkabiir Oct 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petertseng Oct 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devkabiir commented Oct 15, 2017 • edited Loading

petertseng commented Oct 15, 2017

Insti commented Oct 31, 2017

devkabiir commented Nov 2, 2017

ErikSchierboom commented Nov 2, 2017

devkabiir commented Nov 2, 2017

NobbZ commented Nov 3, 2017

petertseng commented Nov 3, 2017

devkabiir commented Nov 3, 2017

petertseng commented Nov 4, 2017

coriolinus commented Nov 5, 2017

coriolinus left a comment

Choose a reason for hiding this comment

Insti commented Nov 21, 2017

devkabiir commented Nov 22, 2017

coriolinus commented Nov 22, 2017

Insti commented Nov 22, 2017

devkabiir commented Nov 23, 2017 • edited Loading

Insti commented Nov 23, 2017

devkabiir commented Nov 25, 2017

petertseng Nov 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devkabiir Dec 6, 2017 • edited Loading

Choose a reason for hiding this comment

devkabiir Dec 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikSchierboom commented Jan 4, 2018

rpottsoh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpottsoh commented Jan 5, 2018

ErikSchierboom commented Jan 5, 2018

Stargator commented Jan 23, 2018

devkabiir commented Oct 10, 2017 •

edited by Insti

Loading

petertseng Oct 10, 2017 •

edited

Loading

devkabiir Oct 10, 2017 •

edited

Loading

devkabiir Oct 10, 2017 •

edited

Loading

petertseng Oct 10, 2017 •

edited

Loading

devkabiir commented Oct 15, 2017 •

edited

Loading

devkabiir commented Nov 23, 2017 •

edited

Loading

petertseng Nov 26, 2017 •

edited

Loading

devkabiir Dec 6, 2017 •

edited

Loading

devkabiir Dec 6, 2017 •

edited

Loading