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

RFC #48 Change String.join to take Iterable #2159

Merged
merged 2 commits into from
Aug 14, 2017
Merged

Conversation

mikeb01
Copy link
Contributor

@mikeb01 mikeb01 commented Aug 12, 2017

Implements RFC #48. For full details see
https://github.com/ponylang/rfcs/blob/master/text/0048-change-String-join-to-take-iterable.md.

The Iterator interface places fewer requirements on the implementation
when compared to ReadSeq. The ReadSeq requires that your implementation
have random access (apply(USize): T), have a known size (size(): USize)
and support sequential (ordered) access by returning an Iterator from
the values() function. Where as the Iterator only requires that the
implementation have sequential access, so is a subset of the
functionality if ReadSeq. There are a number of cases where supporting
an Iterator interface rather than ReadSeq would be desirable. Supporting
a database cursor or a stream from a network service are two that come
immediately to mind.

The other case that crops up is compatibility with other parts of the
pony stdlib. The itertools package which provides a number of useful
functions over unbounded streams of data, similar java streams library.
The specific case I was looking at taking a stream of data, mapping a
transformation over it, then joining the result. Itertools was the only
part of the library that supported that sort of functionality, but
didn't provide a join or reduce function. I found the join function on
String, which required that the call supply a ReadSeq, but internally
only ever needs in Iterator. It seems a simple and obvious change to
make the join function take an Iterator making it a more useful
function. It is trivial to convert from a ReadSeq to an Iterator,
however it is more difficult and has a higher cost to convert in the
other direction.

Implements RFC ponylang#48. For full details see
https://github.com/ponylang/rfcs/blob/master/text/0048-change-String-join-to-take-iterable.md.

The Iterator interface places fewer requirements on the implementation
when compared to ReadSeq. The ReadSeq requires that your implementation
have random access (apply(USize): T), have a known size (size(): USize)
and support sequential (ordered) access by returning an Iterator from
the values() function. Where as the Iterator only requires that the
implementation have sequential access, so is a subset of the
functionality if ReadSeq. There are a number of cases where supporting
an Iterator interface rather than ReadSeq would be desirable. Supporting
a database cursor or a stream from a network service are two that come
immediately to mind.

The other case that crops up is compatibility with other parts of the
pony stdlib. The itertools package which provides a number of useful
functions over unbounded streams of data, similar java streams library.
The specific case I was looking at taking a stream of data, mapping a
transformation over it, then joining the result. Itertools was the only
part of the library that supported that sort of functionality, but
didn't provide a join or reduce function. I found the join function on
String, which required that the call supply a ReadSeq, but internally
only ever needs in Iterator. It seems a simple and obvious change to
make the join function take an Iterator making it a more useful
function. It is trivial to convert from a ReadSeq to an Iterator,
however it is more difficult and has a higher cost to convert in the
other direction.
@SeanTAllen
Copy link
Member

@mikeb01 you have a compilation error with your PR

@SeanTAllen SeanTAllen added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label Aug 14, 2017
@SeanTAllen SeanTAllen merged commit 41a0613 into ponylang:master Aug 14, 2017
ponylang-main added a commit that referenced this pull request Aug 14, 2017
@SeanTAllen
Copy link
Member

Thanks @mikeb01!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants