You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Emmanuel Bourg <eb...@apache.org> on 2013/12/20 18:21:55 UTC

Re: svn commit: r1552745 - /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java

Le 20/12/2013 18:18, ggregory@apache.org a écrit :

> Remove some unnecessary parentheses.

I'd argue they make the code easier to read. Reading bit shifting code
is quite painful, some parentheses help greatly.

Emmanuel


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1552745 - /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java

Posted by sebb <se...@gmail.com>.
On 20 December 2013 17:21, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 20/12/2013 18:18, ggregory@apache.org a écrit :
>
>> Remove some unnecessary parentheses.
>
> I'd argue they make the code easier to read. Reading bit shifting code
> is quite painful, some parentheses help greatly.

In this case, the parentheses were around the whole expression, so did
not change the grouping visually.
In fact removing them here made it easier to read.

> Emmanuel
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1552745 - /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2013-12-20, Gary Gregory wrote:

> I've made some adjustments to this file in SVN. My intention was to not
> change the bit twiddling expressions, my mistake. Now we have:

>             value |= nextByte << (8 * i);

Thanks

        Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1552745 - /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java

Posted by Gary Gregory <ga...@gmail.com>.
On Fri, Dec 20, 2013 at 12:42 PM, Stefan Bodewig <bo...@apache.org> wrote:

> On 2013-12-20, Gary Gregory wrote:
>
> > On Fri, Dec 20, 2013 at 12:21 PM, Emmanuel Bourg <eb...@apache.org>
> wrote:
>
> >> Le 20/12/2013 18:18, ggregory@apache.org a écrit :
>
> >>> Remove some unnecessary parentheses.
>
> >> I'd argue they make the code easier to read. Reading bit shifting code
> >> is quite painful, some parentheses help greatly.
>
>
> > That's why it's only "some" and not "all" unnecessary parentheses.
>
> > IMO:
>
> >    tableSize = (1 << 8);
>
> agree in this case, but not with
>
> -            value |= (nextByte << (8 * i));
> +            value |= nextByte << 8 * i;
>
> <
> http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java?r1=1552740&r2=1552739&pathrev=1552740
> >
>
> at least I need to think about it to be really sure * has precedence
> over <<
>

I've made some adjustments to this file in SVN. My intention was to not
change the bit twiddling expressions, my mistake. Now we have:

            value |= nextByte << (8 * i);

Gary

>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: svn commit: r1552745 - /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2013-12-20, Gary Gregory wrote:

> On Fri, Dec 20, 2013 at 12:21 PM, Emmanuel Bourg <eb...@apache.org> wrote:

>> Le 20/12/2013 18:18, ggregory@apache.org a écrit :

>>> Remove some unnecessary parentheses.

>> I'd argue they make the code easier to read. Reading bit shifting code
>> is quite painful, some parentheses help greatly.


> That's why it's only "some" and not "all" unnecessary parentheses.

> IMO:

>    tableSize = (1 << 8);

agree in this case, but not with

-            value |= (nextByte << (8 * i));
+            value |= nextByte << 8 * i;

<http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java?r1=1552740&r2=1552739&pathrev=1552740>

at least I need to think about it to be really sure * has precedence
over <<

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1552745 - /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java

Posted by Gary Gregory <ga...@gmail.com>.
On Fri, Dec 20, 2013 at 12:21 PM, Emmanuel Bourg <eb...@apache.org> wrote:

> Le 20/12/2013 18:18, ggregory@apache.org a écrit :
>
> > Remove some unnecessary parentheses.
>
> I'd argue they make the code easier to read. Reading bit shifting code
> is quite painful, some parentheses help greatly.
>

That's why it's only "some" and not "all" unnecessary parentheses.

IMO:

   tableSize = (1 << 8);

is just clutter, there is no confusion _possible_ about operator precedence
in this line of code.

Gary


> Emmanuel
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: svn commit: r1552745 - /commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2013-12-20, Emmanuel Bourg wrote:

> Le 20/12/2013 18:18, ggregory@apache.org a écrit :

>> Remove some unnecessary parentheses.

> I'd argue they make the code easier to read. Reading bit shifting code
> is quite painful, some parentheses help greatly.

+1

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org