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

Add array instances #742

Open
rklaehn opened this issue Dec 11, 2015 · 14 comments
Open

Add array instances #742

rklaehn opened this issue Dec 11, 2015 · 14 comments

Comments

@rklaehn
Copy link
Contributor

rklaehn commented Dec 11, 2015

I think there should be array instances. Probably the same as are defined for Vector

@stew
Copy link
Contributor

stew commented Dec 12, 2015

I think that instances for mutable structures belongs in alleycats

@vil1
Copy link

vil1 commented Oct 24, 2017

Hi, coming there from codetriage.

This issue has been open for quite a long time now. AFAIK, there are no Array instance in alleycat yet, so it feels like there is no progress on this issue whatsoever.

Should we :

  • open a similar issue in alleycats?
  • specify what lawful instances are possible for Array (if any) in the present issue?
  • simply close this issue?

@kailuowang
Copy link
Contributor

@vil1 thanks for trying to contribute. I think it will be great to have an array instance in alleycats which is about to move into cats, see #1984

@johnynek
Copy link
Contributor

I am +1 on Array instances. I don’t even think they need to be in Alleycats as long as they are lawful.

@johnynek
Copy link
Contributor

Meta point: I would like a clear law violation yet argument of utility for things in Alleycats. If we have no law violation, but somehow we don’t like it for other reasons we should be able to make a clear policy.

For a long time Try was kept out of cats for some hand wavy argument about functions that throw. But functions that throw make all laws fail so I don’t see that as a valid argument.

Similar to Iterable. Foldable is already lawless and really just a Typeclass generalizing a toList function. I don’t see any law violation of having Foldable for Iterable in cats. I think it kept out because scala FP programmers don’t like subclassing and usually you don’t create Iterables but treat concrete implementations as Iterables.

@tpolecat
Copy link
Member

tpolecat commented Oct 26, 2017

Instances are only lawful for Array if you don't mutate; for Try if you don't throw; and for Future if you don't side-effect. As these are the primary use cases for all three of these structures I don't think it's ethical to include instances in core. We will forever be explaining to people why their code doesn't work.

(The Array issue, in case it's not clear, is that you can't replace arr with arr.map(identity) arbitrarily without changing the meaning of your program because reference identity matters.)

I can't make a good argument about Foldable since it doesn't introduce any nontrivial identities, but my gut feel is that introducing instances for mutable things is likely to sow confusion.

@johnynek
Copy link
Contributor

since the array instances are lawful IF WE DON'T MUTATE, which we won't, I don't see the issue.

@tpolecat
Copy link
Member

Well I mean, the next question is if you're not mutating why are you using Array? If you're doing it for performance then it seems unlikely you're going to want to use these abstract operations. It seems like a marginal use case to me that's going to lead to a lot of confusion.

Also the TypeTag issue is likely to make a bunch of instances impossible to define. Haven't tried.

@johnynek
Copy link
Contributor

johnynek commented Oct 26, 2017 via email

@tpolecat
Copy link
Member

Ok, I'm just trying to understand what you're advocating for. Would you also want instances for ListBuffer for instance? I'm not clear on how big the box is.

@johnynek
Copy link
Contributor

I think someone needs to argue for an instance. To me, Array is common enough that I would like it. I personally don't use ListBuffer, so I wouldn't advocate for that, but I wouldn't push back on it either (as long as the implementation is lawful).

@tpolecat
Copy link
Member

Ok, thanks for clarifying. I remain 👎 for core instances that are invalid when the data type is used as intended. I'm 👍 for alley-cats, but the limitations of these instances should be clearly documented.

@johnynek
Copy link
Contributor

In the case of Array and Foldable, how is it invalid if is you assume Array is mutable?

Are you saying in another thread someone could be mutating Array without holding a lock and then we break the laws? Or are you saying that Array + Foldable seems fine?

@tpolecat
Copy link
Member

tpolecat commented Oct 26, 2017

I think it's valid because there are no real laws, but it's not exactly fine because the operations are all side-effects if you're mutating the array; you can't inline a call to foldMap because the result depends on when you ask the question. To me this is problematic enough to boot the instance to alley-cats, but I may be the only person bothered by this … interested in what others think.

Maybe we should move Foldable to alley-cats. 🔥

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

6 participants