Skip to content

Add optional message to Option.getExn #212

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

Merged
merged 6 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- BREAKING: Use new native `bigint` type. This requires ReScript compiler version "11.1.0-rc.6" or higher. https://github.com/rescript-association/rescript-core/pull/207
- `Int`, `Float`, `BigInt`: use optional args and deprecate `xxxWithRadix`, `xxxWithPrecision` etc. https://github.com/rescript-association/rescript-core/pull/209
- Add optional `~message: string=?` to `Option.getExn`. https://github.com/rescript-association/rescript-core/pull/212

## 1.2.0

Expand Down
9 changes: 4 additions & 5 deletions src/Core__Option.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Generated by ReScript, PLEASE EDIT WITH CARE

import * as Caml_option from "rescript/lib/es6/caml_option.js";
import * as Core__Error from "./Core__Error.mjs";

function filter(opt, p) {
if (opt !== undefined && p(Caml_option.valFromOption(opt))) {
Expand All @@ -16,14 +17,12 @@ function forEach(opt, f) {

}

function getExn(x) {
function getExn(x, message) {
if (x !== undefined) {
return Caml_option.valFromOption(x);
} else {
return Core__Error.panic(message !== undefined ? message : "Not found.");
}
throw {
RE_EXN_ID: "Not_found",
Error: new Error()
};
}

function mapOr(opt, $$default, f) {
Expand Down
10 changes: 8 additions & 2 deletions src/Core__Option.res
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,16 @@ let forEach = (opt, f) =>
| None => ()
}

let getExn = x =>
let getExn = (x, ~message=?) =>
switch x {
| Some(x) => x
| None => raise(Not_found)
| None =>
Core__Error.panic(
switch message {
| None => "Not found."
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the implementation of panic, this means that by default an Error will be raised with the message "Panic! Not found." which sounds a bit weird to me.

Not sure what would be a better message, maybe something like
"Panic! Option.getExn called for None value"
?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, better. Changing.

Copy link
Member

Choose a reason for hiding this comment

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

What about using a custom exception? It could be in a different function though.

| Some(message) => message
},
)
}

external getUnsafe: option<'a> => 'a = "%identity"
Expand Down
5 changes: 3 additions & 2 deletions src/Core__Option.resi
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,19 @@ Option.forEach(None, x => Console.log(x)) // returns ()
let forEach: (option<'a>, 'a => unit) => unit

/**
`getExn(opt)` returns `value` if `opt` is `Some(value)`, otherwise raises an exception.
`getExn(opt, ~message=?)` returns `value` if `opt` is `Some(value)`, otherwise raises an exception with the message provided, or a generic message if no message was provided.

```rescript
Option.getExn(Some(3)) // 3
Option.getExn(None) /* Raises an Error */
Option.getExn(None, ~message="was None!") /* Raises an Error with the message "was None!" */
```

## Exceptions

- Raises an error if `opt` is `None`
*/
let getExn: option<'a> => 'a
let getExn: (option<'a>, ~message: string=?) => 'a

/**
`getUnsafe(opt)` returns `value` if `opt` is `Some(value)`, otherwise `undefined`.
Expand Down
2 changes: 1 addition & 1 deletion test/Test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function print(value) {
if (match === "object" || match === "bigint") {
return Util.inspect(value);
} else if (match === "string") {
return Core__Option.getExn(JSON.stringify(value));
return Core__Option.getExn(JSON.stringify(value), undefined);
} else {
return String(value);
}
Expand Down
10 changes: 5 additions & 5 deletions test/TypedArrayTests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -56,31 +56,31 @@ assertTrue("fromArray", (function () {
return areSame(Core__Option.getExn(new BigInt64Array([
num1,
num2
])[1]), num2);
])[1], undefined), num2);
}));

assertTrue("fromBuffer", (function () {
var x = new BigInt64Array(new ArrayBuffer(16));
x[1] = num2;
return areSame(Core__Option.getExn(x[1]), num2);
return areSame(Core__Option.getExn(x[1], undefined), num2);
}));

assertWillThrow("fromBuffer when too short can throw when used", (function () {
var x = new BigInt64Array(new ArrayBuffer(1));
x[0] = num1;
areSame(Core__Option.getExn(x[0]), num1);
areSame(Core__Option.getExn(x[0], undefined), num1);
}));

assertTrue("fromBufferWithRange", (function () {
var x = new BigInt64Array(new ArrayBuffer(16), 0, 1);
x[0] = num1;
return areSame(Core__Option.getExn(x[0]), num1);
return areSame(Core__Option.getExn(x[0], undefined), num1);
}));

assertWillThrow("fromBufferWithRange is unsafe, out of range", (function () {
var x = new BigInt64Array(new ArrayBuffer(16), 13, 1);
x[0] = num1;
areSame(Core__Option.getExn(x[0]), num1);
areSame(Core__Option.getExn(x[0], undefined), num1);
}));

assertTrue("fromLength is NOT in bytes", (function () {
Expand Down
Loading