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.

Sign up for GitHub

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

Jump to bottom

Make it clearer which end of the Flow history stack is the top #261

Open
zach-klippenstein opened this issue Jun 5, 2017 · 2 comments
Open

Make it clearer which end of the Flow history stack is the top #261

zach-klippenstein opened this issue Jun 5, 2017 · 2 comments

Comments

@zach-klippenstein
Copy link
Collaborator

zach-klippenstein commented Jun 5, 2017 •
edited

It's not obvious that we treat the front of the History list as the top of the stack. You can always dig into the code to find out, but it should be easier to just read without knowing how Flow works under the hood.

I propose deprecating History#iterator() (and History.Builder, and #reverseIterator()) and replace them with something like Iterable<Object> framesTop() and Iterable<Object> History#framesFromBottom().
@zach-klippenstein
Copy link
Collaborator Author

This also makes using reverseIterator much more idiomatic, since you can just use a for each loop.

zach-klippenstein added a commit to zach-klippenstein/flow that referenced this issue Jun 5, 2017
@zach-klippenstein
Deprecate History#iterator() and History#reverseIterator(), use more …
00cab9c 
…explicit methods returning Iterables instead.

The raw `iterator` methods on `History` do not make it clear you're iterating from the top or bottom of the
stack. Introduced new methods `framesFromTop` and `framesFromBottom` to make it very clear. This also lets
you use `for each` loops for both forward and backward iteration.

Fix for issue square#261.
zach-klippenstein added a commit to zach-klippenstein/flow that referenced this issue Jun 5, 2017
@zach-klippenstein
Deprecate History#iterator() and History#reverseIterator(), use more …
03d9578 
…explicit methods returning Iterables instead.

The raw `iterator` methods on `History` do not make it clear you're iterating from the top or bottom of the
stack. Introduced new methods `framesFromTop` and `framesFromBottom` to make it very clear. This also lets
you use `for each` loops for both forward and backward iteration.

Fix for issue square#261.
@zach-klippenstein zach-klippenstein mentioned this issue Jun 5, 2017
Merged
@zach-klippenstein
Copy link
Collaborator Author

Got the method names wrong, will follow up with another PR.

zach-klippenstein pushed a commit to zach-klippenstein/flow that referenced this issue Jun 27, 2017
Swap the names of framesFromTop and framesFromBottom so they're c…
1145318 
…orrect.

I added `framesFromTop` and `framesFromBottom` methods in 03d9578,
but the names of the methods were backwards.

Follow-up for fix for issue square#261.
zach-klippenstein pushed a commit to zach-klippenstein/flow that referenced this issue Jun 27, 2017
Swap the names of framesFromTop and framesFromBottom so they're c…
d1541cb 
…orrect.

I added `framesFromTop` and `framesFromBottom` methods in 03d9578,
but the names of the methods were backwards.

Follow-up for fix for issue square#261.
@zach-klippenstein zach-klippenstein mentioned this issue Jun 27, 2017
Merged
zach-klippenstein pushed a commit to zach-klippenstein/flow that referenced this issue Jul 7, 2017
Swap the names of framesFromTop and framesFromBottom so they're c…
b94f35d 
…orrect.

I added `framesFromTop` and `framesFromBottom` methods in 03d9578,
but the names of the methods were backwards.

Follow-up for fix for issue square#261.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@zach-klippenstein