Added exercise binary-search #351

shybyte · 2017-09-18T05:31:39Z

I have added the binary-search exercise based on https://github.com/exercism/problem-specifications/tree/master/exercises/binary-search.

Additionally I have added 2 bonus tasks, so the students can practice the traits Ord and AsRef if they feel adventurous.

coriolinus · 2017-09-20T17:20:35Z

exercises/binary-search/.meta/hints.md

+[split_at](https://doc.rust-lang.org/std/primitive.slice.html#method.split_at) or [getting 
+subslices](https://doc.rust-lang.org/std/primitive.slice.html#method.get) (slice[start..end]).
+
+Can can solve this exercise by just using boring old element access via indexing, but maybe the


Repeated word: Can can solve this exercise...

Thanks for spotting this typo. I have fixed it now.
In order to get one clean commit I have amended the fix to my last commit. I'm not sure if this is good or bad practice, because it makes probably a review of my fix more complicated for you?

Disclaimer: I'm not a project maintainer, so don't take this as gospel.

What I've seen from the maintainers is that they tend to squash multi-commit PRs into a single commit anyway, so I don't think it matters either way.

In order to get one clean commit I have amended the fix to my last commit. I'm not sure if this is good or bad practice, because it makes probably a review of my fix more complicated for you?

Doesn't matter to me. I check out what I review locally and so I'll always be able to diff PR current contents versus what I last reviewed.

squash multi-commit PRs into a single commit

This must only be done when the PR would make sense as a single commit. If the PR contains two logical changes, the maintainers must not squash.

petertseng

Everything is looking good.

I can fix up the details with regard to the README generation and what needs to go in hints.md.

I need your input about the difficulty and placement. I didn't see why difficulty 7 was in the middle of difficulty 4 exercises

petertseng · 2017-10-10T01:05:57Z

exercises/binary-search/tests/binary-search.rs

+#[test]
+#[ignore]
+fn finds_first_value_in_an_array_with_two_element() {
+    assert_eq!(find(&[1, 2], 1), Some(0));


note, I don't see these in https://github.com/exercism/problem-specifications/blob/master/exercises/binary-search/canonical-data.json , that's fine, but did you add them yourself?

Should they be added to https://github.com/exercism/problem-specifications/blob/master/exercises/binary-search/canonical-data.json ?

I have added them by myself because in the canonical section there is a big step between the 1-element test case and the 7-element test case. For incremental (TDD-wise) coding it's easier to get it first working with an 2-element array (no loops or recursion needed). But maybe this is just my personal thinking style, so I don't know if it should go into the canonical test data.

there is a big step between the 1-element test case and the 7-element test case

That sounds like something that all language tracks can and should benefit from. It is in line with the sentence "manageable increase in complexity" that is noted in https://github.com/exercism/docs/blob/master/language-tracks/exercises/anatomy/test-suites.md. Therefore it should be offered as a PR to problem-specifications. I wouldn't want the Rust track to look selfish (whether or not we actually are selfish)

OK, I will make a PR in the next days.

petertseng · 2017-10-10T01:08:00Z

exercises/binary-search/README.md

@@ -0,0 +1,113 @@
+# Binary Search
+
+Write a program that implements a binary search algorithm.


By exercism/problem-specifications#321, we are removing the "Write a program"

So the whole sentence should be removed?

Just change it to "Implement a binary search algorithm."

So the whole sentence should be removed?

I should have been more clear, therefore I apologise for not being so.

My goal is that the README given in this PR is exactly the README generated by configlet generate (documented at https://github.com/exercism/docs/blob/master/maintaining-a-track/regenerating-exercise-readmes.md), with no modifications. Because of time, the track maintainers are unable to maintain any README that is different from the generated README.

This sentence containing "Write a program" is different from the first sentence of https://github.com/exercism/problem-specifications/blob/master/exercises/binary-search/description.md, therefore it is different from the generated README.

I will be generating the README, so you do not need to fix it yourself, but I need you to know it for future PRs.

petertseng · 2017-10-10T01:09:40Z

exercises/binary-search/README.md

+so locating an item (or determining its absence) takes logarithmic time.
+A binary search is a dichotomic divide and conquer search algorithm.
+
+## Restrictions


This text is going to have go in hints.md, otherwise it disappears when README is regenerated.

Do you mean the Restrictions section? Actually it's not really a hint but a part of the exercise. So I guess it should go into the main exercise text?

In https://github.com/exercism/rust/blob/master/exercises/decimal/README.md a "# note" section is used for similar advice. Is this the right section to put it into?

Do you mean the Restrictions section?

I should have been more clear, therefore I apologise for not being so.

My goal is that the README given in this PR is exactly the README generated by configlet generate (documented at https://github.com/exercism/docs/blob/master/maintaining-a-track/regenerating-exercise-readmes.md), with no modifications. Because of time, the track maintainers are unable to maintain any README that is different from the generated README.

So, the entire section needs to go into hints.md, otherwise it will disappear.

I will be generating the README, so you do not need to fix it yourself, but I need you to know it for future PRs.

petertseng · 2017-10-10T01:09:44Z

exercises/binary-search/README.md

+You can solve this exercise by just using boring old element access via indexing, but maybe the
+other provided functions can make your code cleaner and safer. 
+
+## For bonus points


This text is going to have go in hints.md, otherwise it disappears when README is regenerated.

Do you mean the complete *for bonus points * section? Actually it's not really a hint but a part of the exercise. So I guess it should go into the main exercise text?

In https://github.com/exercism/rust/blob/master/exercises/decimal/README.md a "# note" section is used for additional information. Is this the right section to put it into?

Do you mean the complete *for bonus points * section

I should have been more clear, therefore I apologise for not being so.

My goal is that the README given in this PR is exactly the README generated by configlet generate (documented at https://github.com/exercism/docs/blob/master/maintaining-a-track/regenerating-exercise-readmes.md), with no modifications. Because of time, the track maintainers are unable to maintain any README that is different from the generated README.

So, the entire section needs to go into hints.md, otherwise it will disappear.

I will be generating the README, so you do not need to fix it yourself, but I need you to know it for future PRs.

petertseng · 2017-10-10T01:10:39Z

config.json

+      "slug": "binary-search",
+      "core": false,
+      "unlocked_by": null,
+      "difficulty": 7,


any reason why difficulty 7 exercise is coming between two difficulty 4 exercise?

I was unsure about the position in config.json, because I haven't found a canonical list. So I looked into https://github.com/exercism/ecmascript/blob/master/config.json and could not figure out the logic behind order and difficulty. So I have added binary-search before robot-simulator in the rust version, because it was before robot-simulator in the ecmascript version. If you have a better position, please suggest a change or fix it, I don't have any strong opinions about it.

I think the issue is with the assigned difficulty, not with the position in the list. I don't think an exercise which can be implemented in 30 lines of code in under 30 minutes is of difficulty 7. If you have it in the same relative position as it was in a different language track, the correct thing would be to rate this exercise as a difficulty 4, so it fits in with its peers.

The difficulty of 7 I had also just copied from the ecmascript version, because implementing a binary-search in Rust should should have more or less the same relative difficulty as in ecmascript. The length of the solution is in my opinion no criteria, because some exercises required just a lot of easy code while others can be solved in few lines of difficult code. It's also highly subjective.

Actually when I look at the order/difficulty of existing tasks a lot makes subjective no sense at all for me.
Look for example at alphametics & pig-latin. Both a have a difficulty of 4 and pig-latin comes after alphametics, but alphametics is a my view 10-times harder than pig-latin.

unsure about the position in config.json, because I haven't found a canonical list

Because it may differ by language, it does not seem there will be a canonical list. I wonder if exercism/go#904 will discuss this (they have not yet, therefore you don't need to visit that link)

alphametics & pig-latin. Both a have a difficulty of 4

I can explain alphametics. Everything in "Getting Rusty" in https://github.com/exercism/rust/blob/aea470a76a9c6f0005adebb2a00d969512da3b6e/problems.md was assigned 4. This statement is not to be construed as an endorsement or rejection of the proposition "4 is the correct difficulty".

alphametics is a my view 10-times harder than pig-latin

I agree that alphametics is harder than pig latin. Alphametics should come later.

suggest a change

OK, then my suggestion will be changing the difficulty to 4.

coriolinus · 2017-10-20T11:42:35Z

Sure, I'd agree that it's highly subjective. Still, I'd argue that people learning Rust probably tend to be a bit more "computer-sciency", for lack of a better term, than people learning emcascript; implementing binary search shouldn't be a hard problem for someone like that. One way or another, the immediate goal is to reconcile the configuration. We can set aside the goal of having the exercises in increasing order of difficulty entirely, because we also have another goal for it: the difficulty rankings should increase monotonically. To satisfy that goal, we need to either choose a different difficulty number or reposition the exercise later in the list.

…

On Fri, Oct 20, 2017 at 1:02 PM, shybyte ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In config.json <#351 (comment)>: > @@ -428,6 +428,18 @@ ] }, { + "uuid": "dd74b65c-0d26-4821-9add-064e32e3a5bd", + "slug": "binary-search", + "core": false, + "unlocked_by": null, + "difficulty": 7, The difficulty of 7 I had also just copied from the ecmascript version, because implementing a binary-search in Rust should should have more or less the same relative difficulty as in ecmascript. The length of the solution is in my opinion no criteria, because some exercises required just a lot of easy code while others can be solved in few lines of difficult code. It's also highly subjective. Actually when I look at the order/difficulty of existing tasks a lot makes subjective no sense at all for me. Look for example at alphametics & pig-latin. Both a have a difficulty of 4 and pig-latin comes after alphametics, but alphametics is a my view 10-times harder than pig-latin. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#351 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHdeTvtDuqmC5AtLfTfwKRi_nYmLNm3Yks5suH20gaJpZM4PacpS> .

petertseng

I think this is everything I was thinking about. I'll make sure, but otherwise time to merge.

shybyte · 2017-10-21T12:07:02Z

@petertseng @coriolinus Thanks for all the improvements and advice. I will try to follow them in my next PR from the beginning.

petertseng · 2017-10-26T21:48:29Z

Ah, did I forget to merge this... my mistake.

petertseng · 2017-10-26T21:51:34Z

Thank you for contributing this exercise

coriolinus reviewed Sep 20, 2017

View reviewed changes

shybyte force-pushed the binary-search branch 2 times, most recently from af04bc3 to 8077ec5 Compare September 21, 2017 05:55

Added exercise binary-search

b4f84a3

shybyte force-pushed the binary-search branch from 8077ec5 to b4f84a3 Compare September 21, 2017 06:00

petertseng reviewed Oct 10, 2017

View reviewed changes

petertseng added 6 commits October 20, 2017 11:05

config: binary-search difficulty -> 4

414b768

binary-search: Restrictions/bonus points -> hints.md

e85a33b

binary-search: no "Write a program"

17aa140

binary-search: generated link is the link itself

f837228

binary-search: rm trailing whitespace

9a59f05

binary-search: it's -> its

687fc42

petertseng approved these changes Oct 20, 2017

View reviewed changes

petertseng added 2 commits October 23, 2017 00:45

binary-search: it's -> its in README too

4fce4a6

binary-search: rm an extra newline

16c0b9c

petertseng merged commit 86f2eca into exercism:master Oct 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added exercise binary-search #351

Added exercise binary-search #351

shybyte commented Sep 18, 2017

coriolinus Sep 20, 2017

shybyte Sep 21, 2017

coriolinus Sep 21, 2017

petertseng Sep 21, 2017

petertseng left a comment

petertseng Oct 10, 2017

shybyte Oct 20, 2017

petertseng Oct 20, 2017

shybyte Oct 21, 2017

petertseng Oct 10, 2017

shybyte Oct 20, 2017

coriolinus Oct 20, 2017

petertseng Oct 20, 2017

petertseng Oct 10, 2017

shybyte Oct 20, 2017

shybyte Oct 20, 2017

petertseng Oct 20, 2017

petertseng Oct 10, 2017

shybyte Oct 20, 2017

shybyte Oct 20, 2017

petertseng Oct 20, 2017

petertseng Oct 10, 2017

shybyte Oct 20, 2017

coriolinus Oct 20, 2017

shybyte Oct 20, 2017

petertseng Oct 20, 2017

coriolinus commented Oct 20, 2017 via email

petertseng left a comment •

edited

Loading

shybyte commented Oct 21, 2017

petertseng commented Oct 26, 2017

petertseng commented Oct 26, 2017

		@@ -0,0 +1,113 @@
		# Binary Search

		Write a program that implements a binary search algorithm.

Added exercise binary-search #351

Added exercise binary-search #351

Conversation

shybyte commented Sep 18, 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

petertseng 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coriolinus commented Oct 20, 2017 via email

petertseng left a comment • edited Loading

Choose a reason for hiding this comment

shybyte commented Oct 21, 2017

petertseng commented Oct 26, 2017

petertseng commented Oct 26, 2017

petertseng left a comment •

edited

Loading