Skip to content

Add Binary Search Tree exercise #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

devkabiir
Copy link
Contributor

@devkabiir devkabiir commented Oct 6, 2017

This exercise:

  • Focuses on implementation of a data structure using classes.
  • explains how privacy (private properties and methods) works in dart.
  • has resources and tips section in readme.md.
  • has simple tests.

I left the config.json untouched on purpose.

Copy link
Contributor

@Stargator Stargator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Binary Search Tree specification doesn't have a canonical-data.json. In fact, there's a PR to include it.

We should follow Exercism precedence and wait until the json cases are implemented.

@Stargator
Copy link
Contributor

Stargator commented Oct 9, 2017

Given comments in exercism/problem-specifications#555 (comment)

I think we should figure out how to order this. It's a complicated problem compared to the rest. I think we will need more exercises that can build up to this.

I'm fine with adding it, but we probably should be mindful of the varying difficulties and balance them out as much as we can.

@devkabiir
Copy link
Contributor Author

@Stargator I've made a PR for adding the required canonical data but it's still open and needs opinions. I'd keep this PR open until that one is resolved. And then reorder the tests accordingly. Apart from tests I'd like your review on the implementation.

It's been a hectic week, it's Diwali season in India. Diwali is in 4 days there's all sorts of preparations pending. So my contributions will be slow this week.

@Stargator
Copy link
Contributor

No worries. Thanks.

@Stargator Stargator added this to the New Batch of Exercises milestone Jan 30, 2018
@devkabiir
Copy link
Contributor Author

@Stargator Will start working on this one as soon as i get more time , this probably only needs updating tests, maybe a few changes in the example.dart and until config.json changes are figured out for this one. It'll probably be on standby.

@jvarness
Copy link
Contributor

jvarness commented Apr 7, 2018

@Stargator @devkabiir does it make more sense to do other exercises first that involved data structures but are less complicated than binary trees?

Are there available exercises to implement that deal with like... Maps or Lists?

@Stargator
Copy link
Contributor

@jvarness Probably, we may even already have some that could use List in the solution. But we haven't looked at specific ones that deal with those types explicitly in the problem.

Also, most of the proposed exercise implementations are not from some high-level plan, it's just whatever each person decided to spend time on. We could hold off on the binary tree implementations until we add a few more exercises that may make t easier for newcomers. We just have't had that discussion, yet. This PR could serve well in that regard.

If interested, this repo contains all the specifications for current exercises that can be applied to any track. Each exercise should have a defined specification before a track implements it.

@Stargator Stargator changed the title Add Binary Search Tree exercise [WIP] Add Binary Search Tree exercise Jul 18, 2018
@devkabiir
Copy link
Contributor Author

This probably needs a full re-write (tests and example) as per updated canonical-json and all other major changes, I'll work on this probably next weekend.

@devkabiir devkabiir changed the title [WIP] Add Binary Search Tree exercise Add Binary Search Tree exercise Jan 28, 2019
@devkabiir
Copy link
Contributor Author

Rewrote it compliant with the new canonical-data.json. I left config.json untouched on purpose. We should discuss the that.
@Stargator , @jvarness

@devkabiir devkabiir force-pushed the binary-search-tree branch 2 times, most recently from 62cd7da to 5b77aa5 Compare January 28, 2019 18:55
Copy link
Contributor

@Stargator Stargator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Congratz @devkabiir It's been a long road for you on this!

For config.json:

    {
      "slug": "binary-search-tree",
      "uuid": "597f86bc-6717-4a4c-a2c6-58241ac3d6cb",
      "core": true,
      "unlocked_by": null,
      "difficulty": 5,
      "topics": [
        "inheritance",
        "classes",
"trees", "algorithms", "logic", "searching", "object_oriented_programming"
      ]
    }

@jvarness @devkabiir Note the topics, these are just what I choose from a look at the TOPICS.txt file. Feel free to discuss and bring up your own.

@devkabiir
Copy link
Contributor Author

Need a little help here, Running pub run dart_style:format -l 120 -n . locally gives no output. Neither does pub run dart_style:format -l 120 -w . make any changes. Tired with dart 1.24.3 and dart 2.1. But the build seems to fail. Whats the issue?

@devkabiir
Copy link
Contributor Author

devkabiir commented Feb 28, 2019

Made a few notable changes:

  1. Usage of null-aware operators
  2. Updated code docs
  3. Stricter types on BinarySearchTree and Node classes (removed dynamic)
  4. Moved assert statements to constructor bodies. Keeping it in initializer seems to not work in dart 1.24.3
  5. Added config.json entry, and added few more topics

I wanted to use Exceptions instead of asserts but thought that would be too many topics in a single exercise?

@Stargator
Copy link
Contributor

Stargator commented Feb 28, 2019

@devkabiir From the root directory of the repo, I ran pub run dart_style:format -l 120 -n exercises/binary-search-tree/

And this is the response I got:
test/binary_search_tree_test.dart

I was using Dart 2.0, but 1.24.3 should work too. Though, Dart 1.24.3 didn't give me a response, must be due to differences in the SDK.

To answer your question, the issue seems to be that dart_style is not in binary_search_tree's pubspec.yaml. So use the main repo's and you should be good.

@Stargator
Copy link
Contributor

Hey @devkabiir do you think you can complete this by April 30th? That's the deadline for the New Batch of Exercises milestone

@devkabiir
Copy link
Contributor Author

Closed in #187

@devkabiir devkabiir closed this Apr 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants