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

Implement rotate left and right #61

Merged
merged 3 commits into from
Oct 18, 2018
Merged

Conversation

CyDragon80
Copy link
Contributor

Just a simple implementation of rotate operations using what appears to be the most recommended method these days. It should hopefully satisfy the request in #45.

@CyDragon80 CyDragon80 force-pushed the master branch 2 times, most recently from 3851890 to 7adb89b Compare June 18, 2018 14:09
@dcodeIO
Copy link
Owner

dcodeIO commented Jun 19, 2018

Thanks! One thing I am concerned about is that it might be more efficient to operate on the underlying low and high bits directly and return a new long from the computed bits just once, instead of relying on the high level wrappers that potentially allocate intermediate instances multiple times. Wdyt?

@CyDragon80
Copy link
Contributor Author

It's a fair point. I'll take a look at it.

@CyDragon80
Copy link
Contributor Author

CyDragon80 commented Jun 19, 2018

Perhaps something more along these lines?
I think I got the logic right. (Been on vacation, so my brain might be running in a lower gear.) I certainly added a couple more test cases.

@CyDragon80 CyDragon80 force-pushed the master branch 3 times, most recently from 696a678 to 5ef80d5 Compare June 26, 2018 17:12
@CyDragon80
Copy link
Contributor Author

@dcodeIO When you get a moment, I think I'm ready for more input. Thanks.

@dcodeIO dcodeIO merged commit 8181a6b into dcodeIO:master Oct 18, 2018
@dcodeIO
Copy link
Owner

dcodeIO commented Oct 18, 2018

Thanks, looks good! Merging, and if there are still issues we can continue from here.

@NameFILIP
Copy link

Hi @dcodeIO, are you going to publish a new version containing this commit?

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.

3 participants