Use arrays instead of lists as they're cleaner and faster in our case #21
Conversation
| } else { | ||
| #ok(value) | ||
| let required = { | ||
| Description.names: list{"required"}, |
There was a problem hiding this comment.
I went for a record instead of a tuple as they're not slower and more explicit 👍
| validations->ListExtra.flatMap(Validations.getName) | ||
| props["validations"]->Option.mapWithDefault([], validations => | ||
| validations->ArrayExtra.flatMap(validation => | ||
| validation->Validations.getNames->List.toArray |
There was a problem hiding this comment.
Tried to isolate the conversion code as much as I could.
When ready, we can simply drop the List.toArray function call and use arrays instead of lists
| switch validation({ | ||
| Validations.Validator.Args.label: props["errorLabel"]->OptionExtra.alt( | ||
| switch validator({ | ||
| Validations.Validator.Args.label: props["errorLabel"]->OptionExtra.or( |
There was a problem hiding this comment.
More consistent with the future BeltExtra lib 😋
gaku-sei
force-pushed
the
use-array-instead-of-lists
branch
from
fe8c478 to
98e5fc1
Compare
| @@ -27,26 +27,36 @@ module Validator = { | |||
| type t<'values, 'value, 'error> = Args.t<'values, 'value, 'error> => Value.t<'value, 'error> | |||
| } | |||
|
|
|||
| module Pair = { | |||
| type t<'values, 'value, 'error> = (list<string>, Validator.t<'values, 'value, 'error>) | |||
| module Description = { | |||
There was a problem hiding this comment.
It's not a pair anymore ^^
There was a problem hiding this comment.
I like Record more, since like you said it is clearer with the label
There was a problem hiding this comment.
Thank you for the feedback 😄
| | list{x, ...xs} => x === y ? true : includes(xs, y) | ||
| } | ||
| module ArrayExtra = { | ||
| let flatMap = (xs, f) => xs->Array.reduce([], (acc, x) => acc->Js.Array2.concat(f(x))) |
There was a problem hiding this comment.
I wonder why should we use Js.Array2.concat instead of Array.concat @gaku-sei -san?
There was a problem hiding this comment.
It's pretty simple, Js.Array2.concat is direct bindings to the native Array.prototype.concat method, while Array.concat is a reimplementation.
We might need to see which is better in the future 😕
|
|
||
| module type Values = { | ||
| type t | ||
| } |
There was a problem hiding this comment.
Feel like you are not using it right?
There was a problem hiding this comment.
You're right, I dropped it in the other pr 👍
Notice that validations still take a list! I have a workaround but as it changes the surface API i will push it in an other pr