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

added logical shift operations #45

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

0xflotus
Copy link
Contributor

part 1 of fix #44

@0xflotus 0xflotus changed the title added lshift operations added logical shift operations Oct 13, 2020
* Shifts all given bits to left and returns the resulting bits
*
* @example
* lshiftl([1,0,1,1,0,1]) => [0,1,1,0,1,0]

Choose a reason for hiding this comment

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

@0xflotus can you make a case for this not returning [1,0,1,1,0,1,0] instead? bitwise basically supports arbitrary size bit arrays, so perhaps this is an unexpected result

If you have a particular use-case in mind for the non-size-increasing version, we could also add a flag for this behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are 6 Bits in the example and you are referring to 7 Bits. Which case should it return?

source/bits/lshiftl.ts Outdated Show resolved Hide resolved
* Shifts all given bits to right and returns the resulting bits
*
* @example
* lshiftr([1,0,1,1,0,1]) => [0,1,0,1,1,0]

Choose a reason for hiding this comment

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

Not as concerned about this one, but I suppose it’s possible to argue that the leading 0 should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are you sure?

source/bits/lshiftr.ts Outdated Show resolved Hide resolved
@FlorianWendelborn
Copy link
Owner

@0xflotus I merged the circular shift one as it was obvious how it should behave on arbitrary lengths of bits (doesn’t modify length). However, I’m really not sure about this one and if it’s more reasonable to keep the length or to decrease/increase it by amount

Base automatically changed from master to main March 17, 2021 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend Operation Functionality
2 participants