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

Should Monoid(undefined) return the empty Monoid? #301

Closed
bennypowers opened this issue Jul 25, 2018 · 7 comments
Closed

Should Monoid(undefined) return the empty Monoid? #301

bennypowers opened this issue Jul 25, 2018 · 7 comments

Comments

@bennypowers
Copy link
Collaborator

Observe:

const all = All(undefined) // All true
const nobody = mreduce(All, []) // true

Should monoid constructors return the empty monoid when called with nil ?

A practical implication:

isExclusive(undefined) // false
const isAllExclusive = mreduceMap(All, isExclusive)
isAllExclusive([]) // true

Users could reasonably expect allExclusive([]) to be false, since the array has no 'exclusive' members.

In this case, developers will have to do the following, which strikes me as burdensome:

const isAllExclusive = compose(
  option(false),
  map(mreduceMap(All, isExclusive)),
  safe(not(isEmpty))
)
@evilsoft
Copy link
Owner

So this is a valid construction. It has to do with what is called a Trivial Monoid, it is the what is called a Terminal Object in the Category of Mon (the category of all Monoids). It has to do with the how the functor maps from JS -> Mon. The issue with that function is not the Monoid, but as you noted the use of mreduce, concat and all of that on an empty Array.

So the solution you have is the correct one, math-wise, except you could clean it up by using safeLift like:

// isNonEmptyArray :: a -> Boolean
const isNonEmptyArray =
  and(isArray, not(isEmpty))

// isAllExclusive :: a -> Maybe Boolean
const isAllExclusive = safeLift(
  isNonEmptyArray
  mreduceMap(All, isExclusive),
)

Personally I would keep it in a Maybe until I REALLY need the result.

@evilsoft
Copy link
Owner

evilsoft commented Oct 26, 2018

Okay. So other than those laws on why undefined maps to empty.
That is not the problem here. It is with the fold. So for instance, to see what this code would look like in POJS check this out:

const { isArray, isEmpty, } = require('crocks')

// isACat :: a -> Boolean
const isACat =
  x => !!(x && x.type === 'cat')

// allCats :: a -> Boolean
const allCats = xs =>
  isArray(xs) && !isEmpty(xs)
    ? xs.reduce((acc, x) => acc && isACat(x), true)
    : false

log(
  allCats([])
)

Noice if you remove the check in allCats you get the same answer. In order to get this to work. So it is not a problem with the Monoid, it is how the Monoid folds on an empty list. So you would need to check the list. And in the ADT world that would translate to the code in the above comment.

@evilsoft
Copy link
Owner

Now you could go the Semigroup route, with foldMap, BUT it takes a non-empty list, so you would have to put in a Maybe there as well. So you might as well go with mconcat

@dalefrancis88
Copy link
Collaborator

@evilsoft Do we want to close this? it feels like it's complete

@evilsoft
Copy link
Owner

@dalefrancis88 Been giving this a lot of thought and after careful 🌽sideration, I am 💯 on board with what @bennypowers is saying. I am thinking that this behavior of Monoids should be removed, if undefined hits the Monoid and it is not expected we should handle it according to the Monoid.

In the case of Any and All which utilize coercion, should coerce into a false. All other Monoid should throw if their carrier type cannot deal with the unit.

Thoughts?

@dalefrancis88
Copy link
Collaborator

Sounds good. I'll create a new issue with the details of what needs to be done and reference and collar this issue

@dalefrancis88
Copy link
Collaborator

Closing as this issue has a resolution and the work will be tracked #371

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

3 participants