Skip to content

Fix binary value in comment in test code src/test/java/redis/clients/jedis/commands/jedis/BitCommandsTest.java #3402

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

Merged
merged 1 commit into from
May 11, 2023

Conversation

vector090
Copy link
Contributor

@vector090 vector090 commented May 9, 2023

When first setting "0" (string) to redis, the bits are actually 0011 0000, instead of number 0 with bits 0000 0000.

Goods new is: all the code logic are correct. The problem only exists in the comment.

@vector090 vector090 changed the title Fix binary value in comment Fix binary value in comment in test code src/test/java/redis/clients/jedis/commands/jedis/BitCommandsTest.java May 9, 2023
@vector090
Copy link
Contributor Author

vector090 commented May 9, 2023

For convenience, I have some helper code to print bits and their positions (10-based), which makes bit navigation easier.
e.g. The final bits are:

0011 0001  0000 0100  0000 0000  0000 0000  0000 0001  
0123 4567  8901 2345  6789 0123  4567 8901  2345 6789  
0          1          2          3          4          

Copy link
Contributor

@sazzad16 sazzad16 left a comment

Choose a reason for hiding this comment

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

Same state and comment is also present in following bitposBinary() test method.

Again, both of these tests are available in src/test/java/redis/clients/jedis/commands/unified/BitCommandsTestBase.java as well.

@vector090 Could you please change in all of these places? 3 more places to be exact.

@vector090
Copy link
Contributor Author

Same state and comment is also present in following bitposBinary() test method.

Again, both of these tests are available in src/test/java/redis/clients/jedis/commands/unified/BitCommandsTestBase.java as well.

@vector090 Could you please change in all of these places? 3 more places to be exact.

@sazzad16 Before I do more fixing, I need to understand what the origin code intended to save, a string literal "0" or a string with underlying value 0 ?
Since this part is unit test code, things shouldn't be complicated by using a string "0" (0011 0000). Instead, a simple byte 0 (0000 0000) is more suitable.

If that was true, and set(String, String) shall be demonstrated, we can do:
jedis.set("foo", new String(new byte[]{0}) )

@sazzad16
Copy link
Contributor

@vector090 It seems to be the first one.

@vector090
Copy link
Contributor Author

@vector090 It seems to be the first one.

Code updated.
Please review.

@sazzad16 sazzad16 merged commit 1b7c5cc into redis:master May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants