-
-
Notifications
You must be signed in to change notification settings - Fork 503
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
Alternative: add getAlternativeMonoid #1717
Conversation
@willheslam not sure why the CI is failing, the log is no longer available |
c731953
to
5decaba
Compare
5decaba
to
f9012e8
Compare
I don't remember either, but it's passing now. Thanks for reviewing this! |
Thank you @willheslam |
Hi @willheslam, not sure if you see this on a merged PR but I have the following question: in the implementation return {
concat: (first: HKT<F, A>, second: HKT<F, A>) =>
F.alt(SF.concat(first, second), () => F.alt(first, () => second)),
empty: F.zero()
} both Is there a particular conceptual reason to evaluate the parameters twice? Do you see any potential issue with this? |
This provides a way to lift a semigroup into a monoid via an alternative...
think of it like a generalised version of
Option
'sgetMonoid
, except it applies to anyAlternative
.It also means any module without a specialised
getMonoid
function likeTaskOption
andIOOption
also have a way to produce an amalgamation.Hopefully this is useful and not a duplication of functionality.
Alternatively, should all Alternative-capable modules expose a
getMonoid
function?I did debate adding a way to get an Alternative Semigroup from a Semigroup (and Alternative Monoid from a Monoid) but I realised this had the biggest bang for buck in utilising the capabilities of the underlying Alternative.
I am pretty sure the order of
() => F.alt(first, () => second))
is irrelevant given it's a Semigroup.(If the tests are redundant I'd be happy to rewrite them as showing identical behaviour to
Option
'sgetMonoid
function.)