You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/04/22 04:39:55 UTC
[GitHub] [commons-imaging] arturobernalg opened a new pull request #137: IMAGING-292 - Simplify Bitwise operation
arturobernalg opened a new pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] arturobernalg commented on pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137#issuecomment-825805501
it's clearer result[offset + 0] than result[offset] .... not agree..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] aherbert commented on pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137#issuecomment-825879196
It is more about the visual layout of either side of the operation:
```java
result[offset + 3] = (byte) (value >> 24);
result[offset + 2] = (byte) (value >> 16);
result[offset + 1] = (byte) (value >> 8);
result[offset + 0] = (byte) (value >> 0);
```
becomes:
```java
result[offset + 3] = (byte) (value >> 24);
result[offset + 2] = (byte) (value >> 16);
result[offset + 1] = (byte) (value >> 8);
result[offset ] = (byte) (value);
```
If you keep the formatting using spaces for layout purposes where you have removed redundant operations that makes the code clearer.
If you are going to remove all the `+ 0` occurrences then you should remove all the ` >> 0` and ` << 0` bit shifts which are also redundant.
I believe this:
```java
final int index = (u << (precision * 2))
| (j << (precision * 1))
| (k << (precision * 0));
```
has more information about the code intension than:
```java
final int index = (u << (precision * 2))
| (j << precision)
| k;
```
If you prefer the later then please update the PR to remove all the shifts by zero and maintain alignment of the source for clarity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] arturobernalg edited a comment on pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
arturobernalg edited a comment on pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137#issuecomment-825805501
it's clearer result[offset + 0] than result[offset] ? ... not agree.. IMO add 0. add nose to the operations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] arturobernalg closed pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
arturobernalg closed pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] arturobernalg edited a comment on pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
arturobernalg edited a comment on pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137#issuecomment-825914122
> It is more about the visual layout of either side of the operation:
>
> ```java
> result[offset + 3] = (byte) (value >> 24);
> result[offset + 2] = (byte) (value >> 16);
> result[offset + 1] = (byte) (value >> 8);
> result[offset + 0] = (byte) (value >> 0);
> ```
>
> becomes:
>
> ```java
> result[offset + 3] = (byte) (value >> 24);
> result[offset + 2] = (byte) (value >> 16);
> result[offset + 1] = (byte) (value >> 8);
> result[offset ] = (byte) (value);
> ```
>
> If you keep the formatting using spaces for layout purposes where you have removed redundant operations that makes the code clearer.
>
> If you are going to remove all the `+ 0` occurrences then you should remove all the ` >> 0` and ` << 0` bit shifts which are also redundant.
>
> I believe this:
>
> ```java
> final int index = (u << (precision * 2))
> | (j << (precision * 1))
> | (k << (precision * 0));
> ```
>
> has more information about the code intension than:
>
> ```java
> final int index = (u << (precision * 2))
> | (j << precision)
> | k;
> ```
>
> If you prefer the later then please update the PR to remove all the shifts by zero and maintain alignment of the source for clarity.
agree --> https://github.com/apache/commons-imaging/pull/138. let's simplify
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls edited a comment on pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137#issuecomment-824534825
[](https://coveralls.io/builds/39085353)
Coverage remained the same at 76.509% when pulling **fc262a0c818fbbece3b775f4fb1498ed53fab0ff on arturobernalg:feature/IMAGING-292** into **b9d90863653065f4d02e229e9db13e3477e93e65 on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] arturobernalg closed pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
arturobernalg closed pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] aherbert commented on pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137#issuecomment-825786540
The original code is clearer as to the intention. The compiler will get rid of these anyway and it will have no effect on the runtime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] arturobernalg commented on pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137#issuecomment-825914122
> It is more about the visual layout of either side of the operation:
>
> ```java
> result[offset + 3] = (byte) (value >> 24);
> result[offset + 2] = (byte) (value >> 16);
> result[offset + 1] = (byte) (value >> 8);
> result[offset + 0] = (byte) (value >> 0);
> ```
>
> becomes:
>
> ```java
> result[offset + 3] = (byte) (value >> 24);
> result[offset + 2] = (byte) (value >> 16);
> result[offset + 1] = (byte) (value >> 8);
> result[offset ] = (byte) (value);
> ```
>
> If you keep the formatting using spaces for layout purposes where you have removed redundant operations that makes the code clearer.
>
> If you are going to remove all the `+ 0` occurrences then you should remove all the ` >> 0` and ` << 0` bit shifts which are also redundant.
>
> I believe this:
>
> ```java
> final int index = (u << (precision * 2))
> | (j << (precision * 1))
> | (k << (precision * 0));
> ```
>
> has more information about the code intension than:
>
> ```java
> final int index = (u << (precision * 2))
> | (j << precision)
> | k;
> ```
>
> If you prefer the later then please update the PR to remove all the shifts by zero and maintain alignment of the source for clarity.
agree --> https://github.com/apache/commons-imaging/pull/138
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] coveralls commented on pull request #137: IMAGING-292 - Simplify Bitwise operation
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #137:
URL: https://github.com/apache/commons-imaging/pull/137#issuecomment-824534825
[](https://coveralls.io/builds/39031714)
Coverage remained the same at 76.509% when pulling **fa34557e3b3c02eec91bf1223691ec1583c8bc0b on arturobernalg:feature/IMAGING-292** into **c24b1a67780351451c295e3580afbfee41657a99 on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org