Skip to content

exercises(binary-search): return an optional, not an error union#259

Merged
ee7 merged 2 commits into
exercism:mainfrom
ee7:exercises-binary-search-return-optional
Mar 15, 2023
Merged

exercises(binary-search): return an optional, not an error union#259
ee7 merged 2 commits into
exercism:mainfrom
ee7:exercises-binary-search-return-optional

Conversation

@ee7

@ee7 ee7 commented Mar 13, 2023

Copy link
Copy Markdown
Member

Before this commit, binary-search required the user to return an error when the array of input items:

  • was empty
  • or did not contain the target

Return an optional instead, like std.sort.binarySearch. Now, an empty input array is just another case of "value not found".

Also rename buffer to items for similar consistency with upstream. Elsewhere we have used the name buffer only for a
variable that is mutated by a function.

Refs: #229

@ErikSchierboom ErikSchierboom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Much better!

ee7 added 2 commits March 15, 2023 10:24
Before this commit, `binary-search` required the user to return an error
when the array of input items:

- was empty
- or did not contain the target

Return an optional instead, like `std.sort.binarySearch` [1]. Now, an
empty input array is just another case of "value not found".

Also rename `buffer` to `items` for similar consistency with
upstream [1], and to reserve the name `buffer` for a variable that is
mutated by a function.

[1] https://github.com/ziglang/zig/blob/adc6dec26b8b/lib/std/sort.zig#L10

Refs: exercism#229
@ee7 ee7 force-pushed the exercises-binary-search-return-optional branch from 5069d46 to 531ff34 Compare March 15, 2023 09:25
@ee7 ee7 marked this pull request as ready for review March 15, 2023 09:28
@ee7 ee7 merged commit a60a991 into exercism:main Mar 15, 2023
@ee7 ee7 deleted the exercises-binary-search-return-optional branch March 15, 2023 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants