basic result type and functions #3
Conversation
tsloughter
force-pushed
the
result-type
branch
from
1d8395b to
5248e70
Compare
src/result.alp
Outdated
| import_type option.option | ||
|
|
||
| export_type result | ||
|
|
|
@tsloughter sorry to leave you hanging on this! Aside from some missing exports I think this is awesome. Thanks! |
| Some x -> Ok x | ||
| | None -> Err e | ||
|
|
||
| let andThen callback (Ok value) = callback value |
There was a problem hiding this comment.
Here we are using camelCase while everywhere else we are using snake_case. Could we be consistent?
There was a problem hiding this comment.
Have we decided which case we prefer? I'm leaning towards camelCase.
There was a problem hiding this comment.
iirc @lepoetemaudit and I discussed this mid-way through last year and settled on moving to camelCase. I'm easy either way tbh. Other opinions?
There was a problem hiding this comment.
Fab, I'll update the other modules/PR.
There was a problem hiding this comment.
For option this is called flatMap. Which name do we prefer? I took flatMap from Scala as we had option which felt Scala-y to me.
There was a problem hiding this comment.
I prefer andThen personally, but would also change it in option, hehe. So if it is staying that way in option then to be consistent I'll change it here.
Maybe in scala the option and result types are actually related to sequences and so a flatMap makes sense? To me it doesn't fit in the alpaca case and would be confusing.
There was a problem hiding this comment.
It's the result of composing flatten : m m a -> m a and map, so I think it makes sense.
I find the name andThen a little odd, doesn't seem to fit with map, fold etc
src/result.alp
Outdated
| let of_option opt e = | ||
| match opt with | ||
| Some x -> Ok x | ||
| | None -> Err e |
There was a problem hiding this comment.
Could we can this from_option? That name is more common everywhere but OCaml and I think it is easier to discover as the name is closer to spoken English.
|
no worries, thanks for the comments, I'll look at this over the weekend. |
tsloughter
force-pushed
the
result-type
branch
2 times, most recently
from
fe3ccd6 to
3a02c90
Compare
|
Updated. No reason to have rebar.lock in the gitignore, I think it is just confusing to do so? But I can add that back, it conflicted with my adding _checkouts to the ignore so I had to merge them. |
tsloughter
force-pushed
the
result-type
branch
from
3a02c90 to
bc692c8
Compare
|
The lock isn't used for libraries, right? Is it best practice to gitignore it or not? |
|
@lpil it is used. The lock is always used because of the way rebar3 does dependency resolution, http://www.rebar3.org/docs/dependencies , which is the maven model. |
|
Cool, thanks for the link :) |
|
Ready to merge? |
No description provided.