[ 
https://issues.apache.org/jira/browse/IMAGING-292?focusedWorklogId=588105&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-588105
 ]

ASF GitHub Bot logged work on IMAGING-292:
------------------------------------------

                Author: ASF GitHub Bot
            Created on: 23/Apr/21 19:42
            Start Date: 23/Apr/21 19:42
    Worklog Time Spent: 10m 
      Work Description: 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:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 588105)
    Time Spent: 1h 10m  (was: 1h)

> Simplify Bitwise operation
> --------------------------
>
>                 Key: IMAGING-292
>                 URL: https://issues.apache.org/jira/browse/IMAGING-292
>             Project: Commons Imaging
>          Issue Type: Sub-task
>            Reporter: Arturo Bernal
>            Priority: Minor
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> I fully understand if not everyone agrees with this change. The idea is to 
> simplify the code and avoid redundant operations



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to