Hi Oli,
I agree with you. (2) will be simple and can prevent potential risk.
2008/5/21 Oliver Deakin <[EMAIL PROTECTED]>:
> Hi Mark,
>
> Agreed that this could cause problems in the future if it is called from
> another method expecting it to behave properly for entries that do not exist
> in the HashMap. Three options immediately spring to mind:
> 1) Leave it as is - we won't hit the non-existent entry problem at the
> moment due to the method caller always ensuring the entry exists.
> 2) Add a condition after the while loop to return without altering the
> HashMap if m.next is null.
> 3) Get rid of the removeEntry() method by inlining it into the calling
> method and fixing it up to handle non-existent entries.
>
> I think (2) is a simple fix, although (3) is also reasonable since there is
> only a single call to this method. I think (1) is, as you say, a problem
> waiting to happen.
> Personally I'd go with (2).
>
> Regards,
> Oliver
>
>
> Mark Hindess wrote:
>
>> While reviewing Aleksey's improvements in HARMONY-5791, I noticed the
>> following lines in HashMap.java (line 682):
>>
>> final void removeEntry(Entry<K, V> entry) {
>> int index = entry.origKeyHash & (elementData.length - 1);
>> Entry<K, V> m = elementData[index];
>> if (m == entry) {
>> elementData[index] = entry.next;
>> } else {
>> [1] while (m.next != entry && m.next != null) {
>> m = m.next;
>> }
>> [2] m.next = entry.next;
>>
>> }
>> modCount++;
>> elementCount--;
>> }
>>
>> The while loop at [1] has two conditions. The first relates to finding
>> the entry and the second relates to getting to the end of the linked
>> list (i.e. not finding the entry). Executing the line [2] only really
>> makes sense if the first condition was met; if the second was met then
>> it might corrupt the list.
>>
>> As it happens, the only caller of this method in HashMap.java does
>> ensure that the entry is present in the HashMap and thus in the linked
>> list but it still seems like a problem waiting to happen (or at the very
>> least a redundant condition on a while loop).
>>
>> It isn't broken so I'm not sure what (if anything) to "fix".
>>
>> Regards,
>> -Mark.
>>
>>
>>
>>
>>
>
> --
> Oliver Deakin
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
> PO6 3AU
>
>
--
Best Regards,
Jim, Jun Jie Yu
China Software Development Lab, IBM
