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

1627 replace seq #1664

Merged
merged 3 commits into from
Apr 6, 2018
Merged

Conversation

jaapterwoerds
Copy link
Contributor

As suggested in #1627, I replaced the usage of Seq by IList.

@tonymorris
Copy link
Member

👍

It might also be worth writing a document making it clear why Seq has no use-cases.

@hrhino
Copy link
Member

hrhino commented Mar 6, 2018

Would it make sense to replace some of those with arbitrary Foldable/Traversable types? An IList is O(n) to construct if you happen to only have a Foldable lying around.

@jaapterwoerds
Copy link
Contributor Author

jaapterwoerds commented Mar 9, 2018

@tonymorris Good point, but I'm not sure where this would fit in the project and or if it it should be part of this PR. I am for giving the good example in the Scalaz code base and example and test and remove undesirable structures further.

@hrhino I will take a look to see if I use the abstraction instead of concrete IList some cases.

Copy link
Contributor

@emilypi emilypi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tonymorris
Copy link
Member

@jaapterwoerds yeah not in this PR. Perhaps open an issue for creating a document that specifies "the useful subset of scala" so that others can see it, and understand some of the reasoning.

Just a suggestion.

@hrhino
Copy link
Member

hrhino commented Mar 30, 2018

FWIW just this week I wanted to ISet.unions an ISet of ISets. (Obviously it can't make a Monad so flatMap in general was out.) Making an IList of it would have been wasteful.

@fommil
Copy link
Contributor

fommil commented Apr 5, 2018

are we good to merge?

@jaapterwoerds
Copy link
Contributor Author

@hrhino I've spent some time trying to figure out if I could replace IList by Foldable or Traversable but could not find any way.

@fommil I don't have anything else to add so we're good AFAICS

@fommil fommil merged commit d3a340b into scalaz:series/7.3.x Apr 6, 2018
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

Successfully merging this pull request may close these issues.

5 participants