Re: Clarifying PersistentByteMap (was: Re: [patch] Fix PR java/22189)

[Andrew Haley <aph at redhat dot com>]

Robin Green writes:
 > On Mon, Jun 27, 2005 at 10:40:15AM +0100, Andrew Haley wrote:
 > > Robin Green writes:
 > >  > Hi,
 > >  > 
 > >  > Below is a copy of my fix and cleanup for
 > >  > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22189
 > >  > 
 > >  > The actual fix is only one line, but I attempted to rename
 > >  > stuff to clarify the code. Please see the bug report for explanation.
 > >  > 
 > >  > Needs testing and applying.
 > > 
 > > Seems reasonable, but:
 > > 
 > >  - Mixes substantive changes and reformatting.
 > 
 > OK, I've posted the bug fix separately. This email deals with the field rename.
 > 
 > The purpose of renaming the capacity field to internal_capacity is
 > to clarify (for the benefit of newcomers reading the code) that there
 > are two measures of capacity with different values:
 > 
 > 1. The "external_capacity", which is what clients of the PersistentByteMap
 > class see: the maximum number of elements that are allowed to be inserted.
 > The bug was that this external capacity changed from 1 to 0 due to a
 > rounding error.
 > 
 > 2. The "internal_capacity", which is the actual number of slots in the
 > table, and is at least (external_capacity*3)/2.
 > 
 > It could be clearer, I know.
 > 
 > >  - Replaces correctly formatted code with differently formatted code
 > >    for no apparent reason;
 > 
 > I probably had mismatching tab/indent settings in Eclipse - hopefully this time
 > indentations have not been changed. 
 > 
 > > at one point a pair of braces is removed
 > >    and replaced by double-spaced blank lines.
 > 
 > I've reverted that unnecessary change.
 > 
 > >  - A new local variable, realCapacity, is introduced, but it doesn't
 > >    seem to do anything useful.
 > 
 > I've reverted that unnecessary change. I was trying to preserve the
 > optimisation of using a local variable rather than reading from an instance
 > variable several times (I've changed it to read from internal_capacity because
 > it is dealing with the internal capacity there, not the external capacity).

This is also fine with a ChangeLog.

Andrew.