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

Signature of Enum.combine #577

Open
my-little-repository opened this issue Jul 17, 2014 · 6 comments
Open

Signature of Enum.combine #577

my-little-repository opened this issue Jul 17, 2014 · 6 comments

Comments

@my-little-repository
Copy link

Shouldn't the signature of Enum.combine be 'a BatEnum.t -> 'b BatEnum.t -> ('a * 'b) BatEnum.t instead of 'a BatEnum.t * 'b BatEnum.t -> ('a * 'b) BatEnum.t ?

This prevents currying and is in stark contrast with the other combine functions:

LazyList.combine 'a LazyList.t -> 'b LazyList.t -> ('a * 'b) LazyList.t
BatList.combine 'a list -> 'b list -> ('a * 'b) list
BatSeq.combine 'a BatSeq.t -> 'b BatSeq.t -> ('a * 'b) BatSeq.t

I think that combine is a function that is interesting to curry. For example, with a curried Enum.combine function, an indexedEnum function can be defined as follows: indexedEnum = combine (range 1) (compare with Haskell zip [1..]).

@UnixJunkie
Copy link
Member

While it seems that you are correct, changing this would break compatibility with existing code.
The escape route looks like you have to define your own function that you can
curry and that will call Enum.combine at the end.

@hcarty
Copy link
Contributor

hcarty commented Jul 18, 2014

Do we want to change this for 3.0?

@my-little-repository
Copy link
Author

I understand, I can indeed define zip x y = combine (x,y). This is a suggestion for some future release of the batteries library.

@gasche
Copy link
Member

gasche commented Jul 18, 2014

I agree with changing this for 3.0.

@thelema
Copy link
Member

thelema commented Jul 18, 2014

Me too; I've also been bothered that this function takes a tuple.

E.
On Jul 18, 2014 12:26 PM, "gasche" notifications@github.com wrote:

I agree with changing this for 3.0.


Reply to this email directly or view it on GitHub
#577 (comment)
.

@UnixJunkie
Copy link
Member

#578

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

5 participants