Skip to content
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

exercises: consider replacing some errors with optionals/asserts/assumes #229

Open
ee7 opened this issue Mar 7, 2023 · 0 comments
Open
Assignees

Comments

@ee7
Copy link
Member

ee7 commented Mar 7, 2023

For example, for binary-search, the Zig track currently requires the user to return an error when the input is empty:

test "nothing is found in an empty array" {
try testing.expectError(SearchError.EmptyBuffer, binarySearch(u64, 13, &[_]u64{}));
}

or the value is not found:

test "identifies that a value is not included in the array" {
try testing.expectError(SearchError.ValueAbsent, binarySearch(i32, 7, &[_]i32{ 1, 3, 4, 6, 8, 9, 11 }));
}

But we can consider the empty array to be a normal case of "value not found", in which case the only possibilities are "value found, or value not found". Then we can return an optional. The Zig stdlib does that - the implementation of binarySearch returns ?usize

I'll propose changing this one. But we should look at all the exercises and think about whether it's better to return an optional, rather than an error union. Especially exercises where the error set is of length 1.


As of 2023-03-16, the exercises that can return a custom error are:

We previously had:

I might propose removing more custom errors.

@ee7 ee7 self-assigned this Mar 12, 2023
ee7 added a commit to ee7/exercism-zig that referenced this issue Mar 12, 2023
We simplified the error set to contain only one error [1], so there's an
argument to use an optional instead.

[1] 4a4665a exercises(triangle): simplify error set (2023-03-12)

Refs: exercism#229
ee7 added a commit to ee7/exercism-zig that referenced this issue Mar 13, 2023
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 added a commit that referenced this issue Mar 15, 2023
Changes:

- Make `buffer` no longer an optional. This means we now never return
  `SearchError.NullBuffer`, but there was no test case for that.

- Make the implementation more standard, and avoid overflow when
  calculating `mid`.

I intend to later make this exercise return an optional, not an error
union.

Refs: #229
ee7 added a commit that referenced this issue Mar 15, 2023
The tests do not have a test case for a null `buffer`.

This commit is similar to the previous commit, but is user-facing.

Refs: #229
ee7 added a commit to ee7/exercism-zig that referenced this issue Mar 15, 2023
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 added a commit that referenced this issue Mar 15, 2023
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` does [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]. Elsewhere we have used the name `buffer` only for a
variable that is mutated by a function.

[1] https://github.com/ziglang/zig/blob/4414f9c46e77/lib/std/sort.zig#L7-L13

Refs: #229
@ee7 ee7 changed the title exercises: consider using errors less, and optional more exercises: consider replacing some errors with optionals/asserts/assumes Apr 8, 2023
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

No branches or pull requests

1 participant