Add practice exercise binary-search-tree

[jiegillet]

I've been away for a while, but I'm trying to get back in.
Practice exercises usually motivate me, so here is a nice little one :)

[ceddlyburge]

This all looks good. Presumably the copy and tests are all standard and shared between the other languages so don't need to be examined in too much detail?

[ceddlyburge]

@pwadsworth, do you want to give this a review? It seems like you are keen to get involved, and reviewing stuff is very useful for us, and a good way of getting familiar with the codebase and the (sometimes unwritten) conventions and things like that.

I remember Jie reviewing one of my PRs before he became a maintainer, and obviously we all review each others code all the time.

[jiegillet]

This all looks good. Presumably the copy and tests are all standard and shared between the other languages so don't need to be examined in too much detail?

Mostly, but I changed the function names, I thought the canonical ones weren't very good: data -> makeTree and sortedData -> sort.

[ceddlyburge]

Welcome back! I agree about the canonical names. Is it worth raising a PR in the repo to update them for everybody?

[pwadsworth]

@pwadsworth, do you want to give this a review? It seems like you are keen to get involved, and reviewing stuff is very useful for us, and a good way of getting familiar with the codebase and the (sometimes unwritten) conventions and things like that.

Love to. I should have some time to go over it later today.

[pwadsworth]

This looks pretty good to me. My only recommendations are to adapt some of the terms to match Elm concept of no null values and immutability i.e. using the term parameter/argument instead of variable.

[pwadsworth]

I think this needs to be adapted to Elm to avoid the invalid code (list with multiple types), and the use of nil since a big part of Elm is the deliberate effort not to use it.

[ceddlyburge]

This is a good point I think, and happens because this copy comes from the canonical source. I'm not even sure that line 9 adds much anyway, so maybe we could just remove it.

[jiegillet]

We can't modify the instructions, they are standard for all tracks. At best, we can add an addendum, but I don't think it's worth it, many exercises are very vague, and in Exercism, tests are king.

[pwadsworth]

Recommend renaming Nil --> Empty to avoid confusion with Elm not supporting the concept of nil.

[ceddlyburge]

I think Nil in a custom type is fairly idiomatic in Elm, although I'm not quite sure. Nothing is used within Maybe, and other modules tend to use something else to avoid a name clash as Maybe is so common, and I think Nil is often what people go for.

[pwadsworth]

...Nothing is used within Maybe, and other modules tend to use something else to avoid a name clash as Maybe is so common, and I think Nil is often what people go for.

I was recommending a custom type 'Empty' for that reason. Either way, I don't think it's a significant point, just a more idiomatic option.

[jiegillet]

I also thought Nil was quite idiomatic in Elm, but most tutorials I could find in my extensive 5-minute search reported using Empty. Maybe it's more common in Haskell?
Anyway, made the change in d09f956

[pwadsworth]

Recommend renaming Nil --> Empty to avoid confusion with Elm not supporting the concept of nil.

[ceddlyburge]

Hi @jiegillet , I think this all looks good, do you want to merge it whenever you are happy?