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

refactored plus #17

Merged
merged 2 commits into from Aug 27, 2012

Conversation

Projects
None yet
3 participants
@mosesn
Contributor

mosesn commented Aug 26, 2012

motivation

I was reading algebird and had some trouble understanding what exactly was going on in plus, although the behavior is fairly simple, so I refactored it.

plus behavior

There are three different subclasses of RightFolded, and the two arguments to plus can be either of the two. There are nine different possible combinations, because left and right are not interchangeable. Assuming the current version is correct, my refactoring must have the same behavior for all nine of those cases.

covering all nine cases

The left RightFolded reduces to three cases. In two of those cases, RightFoldedValue (RFV) and RightFoldedZero (RFZ), we don't care what the value of the right RightFolded is, because it does the same thing every time. For RFZ, we always want the right value (if right is also RFZ, it doesn't matter which we hand back), and for RFV, we always want the left value. The only cases that remain are the left is RightFoldedToFold (RFTF) cases, where each of those cases is distinct, so I separated those out each to their own call. Hence, there are 6 cases which are covered by the first two case statements, and 3 cases which are covered individually by the nested case statement.

different behavior

No behavior has changed.

comment change

Fixed a typo.

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Aug 26, 2012

This pull request passes (merged 53fdae7 into 6570534).

travisbot commented Aug 26, 2012

This pull request passes (merged 53fdae7 into 6570534).

@johnynek

This comment has been minimized.

Show comment
Hide comment
@johnynek

johnynek Aug 27, 2012

Collaborator

Thanks!

Collaborator

johnynek commented Aug 27, 2012

Thanks!

johnynek added a commit that referenced this pull request Aug 27, 2012

@johnynek johnynek merged commit 631dfd3 into twitter:develop Aug 27, 2012

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment