Group invert behavior

[ghost]

Missing behavior of invert method in Group added.

[Avaq]

I don't think this change is valid. We don't consider the possibility for invert to return anything other than a value of the same Group. That possibility is only considered for user supplied functions. In the case of invert, if the return value is not from the same group, the implementation is simply not FL compliant.

With this change it's like we're saying: "If you don't implement invert according to the spec, then it's behaviour is not according to the spec". - It's true, of course, but a bit redundant. ;)

[evilsoft]

@remisa-yousefvand I wrestled with that as well, and added and deleted the same phrasing over and over again. And finally came to the same 🌽-clusion that @Avaq stated in their comment.

[ghost]

@evilsoft I understand you and @Avaq point of view as a formal documentation where redundancy is prohibited. But if someone jumps in the middle of document to read about a specific item, redundancy may help a lot. That's the way ordinary ppl like me read documentations :book:

[Avaq]

It's not just the redundancy I'm worried about. Making this change will cause it to seem as though the specification somehow tolerates invert implementations that deviate from the spec in terms of their output type.

invert must return a value of the same Group. Since it's up to the library author (as opposed to the library user), the specification considers only the valid implementation. An invalid implementation would deviate from the spec.

---

Note also:

It is recommended to throw an exception on unspecified behaviour. -- source

When we're talking about the output type of an operation, that doesn't make sense - you either return a non-conformant value, or throw an exception, but not both.

I mention this in an attempt to make clearer the intent of the "... behaviour is unspecified" clauses.

If we examine the map spec, for example, we'll see in section 1.1 that "behaviour is unspecified" when f is not a function. That's because f is an argument supplied by the user, and since the user supplies it, the spec cannot strictly determine its type, and recommends throwing an exception. In section 2, however, the behaviour is assumed to always conform to the spec. That's because map is not supplied by the user, but rather is a static method supplied to the user.

[Avaq]

@remisa-yousefvand I agree that the spec is not very clear in the parts around "unspecified behaviour". I think your idea of making this part of the spec more clear and consistent is good. It's just that I don't think this particular change is a step in the right direction.

It might be worth to consider making this kind of change instead:

 A value which has a Functor must provide a `map` method. The `map`
 method takes one argument:

     u.map(f)

-1. `f` must be a function,
+1. `f` should be a function,

-    1. If `f` is not a function, the behaviour of `map` is
-       unspecified.
     2. `f` can return any value.
     3. No parts of `f`'s return value should be checked.
+
+2. If a non-conformant value is supplied for `f`, the
+   behaviour of `map` is unspecified.

 2. `map` must return a value of the same Functor

That way, the spec is more clear about these values being "supplied", and specs for methods like ap are also less inconsistent; I'm referring to the missing "if b is not the same Apply as a, behaviour of ap is unspecified" in section 1.2.

[davidchambers]

It's not just the redundancy I'm worried about. Making this change will cause it to seem as though the specification somehow tolerates invert implementations that deviate from the spec in terms of their output type.

I find this argument compelling.