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/07/02 12:55:57 UTC

[GitHub] [commons-imaging] kinow commented on a change in pull request #155: Remove redundant local variable.

kinow commented on a change in pull request #155:
URL: https://github.com/apache/commons-imaging/pull/155#discussion_r662989639



##########
File path: src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java
##########
@@ -1003,7 +1003,6 @@ public void writeImage(final BufferedImage src, final OutputStream os, Map<Strin
                 // boolean LocalColorTableFlag = false;
                 final boolean interlaceFlag = false;
                 final boolean sortFlag = false;
-                final int sizeOfLocalColorTable = colorTableScaleLessOne;

Review comment:
       I think the author here intentionally made a redundant variable so other developers would understand the code more easily. Imaging has a lot of code with many variables, or methods implementing papers and using the variables as they appear in the papers.
   
   So any help like this helps, so IMO we should keep issues like that when found by static analyzers / linters.

##########
File path: src/main/java/org/apache/commons/imaging/formats/jpeg/decoder/Dct.java
##########
@@ -282,15 +282,14 @@ public static void inverseDCT8(final float[] vector) {
         final float n1 = vector[0] - vector[4];
         final float n2 = b2 - a3;
         final float n3 = vector[0] + vector[4];
-        final float neg_n5 = neg_b4;
 
         // A2
         final float m3 = n1 + n2;
         final float m4 = n3 + a3;
         final float m5 = n1 - n2;
         final float m6 = n3 - a3;
-        // float m7 = n5 - n0;
-        final float neg_m7 = neg_n5 + n0;
+        // float m7 = b4 - n0;
+        final float neg_m7 = neg_b4 + n0;

Review comment:
       Quite sure these variables were copied from a paper or specification document. I would prefer to leave as in the document (if that's the case), but will have to take a better look later to verify if that's really the case.




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org