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

Use CharSequence instead of String for more flexibility #39

Closed
wants to merge 1 commit into from

Conversation

cvogt
Copy link

@cvogt cvogt commented Sep 21, 2015

This e.g. allows using a lazy sequence for streamed parsing. Replaced eager .length calls with less eager charAt calls where needed. Added test that asserts laziness.

fixes #18

This e.g. allows using a lazy sequence for streamed parsing. Replaced eager .length calls with less eager charAt calls where needed. Added test that asserts lazyness.
if (index >= input.length) fail(cfg.failure, index)
else if (uberSet(input(index))) success(cfg.success, (), index + 1, Nil, false)
if (Try(input.charAt(index)).isFailure) fail(cfg.failure, index)
else if (uberSet(input.charAt(index))) success(cfg.success, (), index + 1, Nil, false)
Copy link
Author

Choose a reason for hiding this comment

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

@lihaoyi not completely happy with having an intermediate exception being generated here, but .length is too eager. any better ideas?

@cvogt
Copy link
Author

cvogt commented Oct 5, 2015

bump :)

@lihaoyi
Copy link
Member

lihaoyi commented Oct 5, 2015

Sorry, haven't taken a serious look yet!

The Try(input.charAt(index)).isFailure pattern probably won't do; that will cut into performance a great deal will all the extra allocations of Trys and thunks. I don't know what a better solution would be. Perhaps using our own custom structs instead of CharSequence/Stream[Char] that provides more convenient/performant access to check if something is available?

@cvogt
Copy link
Author

cvogt commented Oct 5, 2015

@lihaoyi yeah, maybe we need our own CharSequence :-. Would be good to measure though first what the performace impact of the Try or maybe using try{}catch{} actually is. You have performance tests, right?

@lihaoyi
Copy link
Member

lihaoyi commented Oct 5, 2015

@lihaoyi
Copy link
Member

lihaoyi commented Nov 8, 2015

Closing due to inactivity

@lihaoyi lihaoyi closed this Nov 8, 2015
@cvogt cvogt mentioned this pull request Jun 24, 2016
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.

Parse CharSequence rather than String
2 participants