RFR (S): 8202021: Improve variable naming in ReferenceProcessor

[sangheon.kim]

Hi Thomas,

On 04/23/2018 07:33 AM, Thomas Schatzl wrote:
> On Mon, 2018-04-23 at 16:02 +0200, Stefan Johansson wrote:
>> Hi Thomas,
>>
>> On 2018-04-19 22:08, Thomas Schatzl wrote:
>>> Hi Sangheon,
>>>
>>>     thanks for your review. I updated the change in-place.
>>>
>>> Thomas
>>>
>>> On Thu, 2018-04-19 at 12:38 -0700, sangheon.kim wrote:
>>>> Hi Thomas,
>>>>
>>>> On 04/19/2018 12:01 PM, Thomas Schatzl wrote:
>>>>> Hi all,
>>>>>
>>>>>      can I have reviews for this cleanup change that improves
>>>>> variable
>>>>> naming in ReferenceProcessor code.
>>>>>
>>>>> The main change is about renaming some member variables from
>>>>> something
>>>>> really confusing to me to something useful (to me again :)) in
>>>>> the
>>>>> DiscoveredListIterator class.
>>>>>
>>>>> The other change is related to changing
>>>>> ReferenceProcessor::num_q
>>>>> to
>>>>> num_queues, i.e. spelling out what the "q" is supposed to mean.
>>>>>
>>>>> That imho improves readability quite a bit.
>>>>>
>>>>> CR:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8202021
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~tschatzl/8202021/webrev/
>> Thanks for doing this, having better names really help when working
>> with this code. The change looks ok, but I still struggle a bit with
>> what all members are. I think my problem is seeing that _ref and
>> _discovered_addr go together. What do you think about renaming:
>> _ref -> _current
>> _discovered_addr -> _current_discovered_addr
> Done.
>
>> Potentially also:
>> _referent -> _current_referent
>> _referent_addr -> _current_referent_addr
>>
> I think the latter renaming is unnecessary as there is no
> prev/current/next distinction.
>
> http://cr.openjdk.java.net/~tschatzl/8202021/webrev.1/ (full)
> http://cr.openjdk.java.net/~tschatzl/8202021/webrev.0_to_1/ (diff)
webrev.1 still looks good.

Thanks,
Sangheon


>
> Thanks,
>    Thomas