Li Lu updated YARN-3836:
    Attachment: YARN-3836-YARN-2928.002.patch

Hi [~sjlee0], thanks for the prompt feedback! I updated the patch according to 
your comments. Specifically:

bq. What I would prefer is to override equals() and hashCode() for Identifier 
instead, and have simple equals() and hashCode() implementations for 
TimelineEntity that mostly delegate to Identifier. The rationale is that 
Identifier can be useful as keys to collections in its own right, and thus 
should override those methods.
That's a nice suggestion! Fixed. 

bq. One related question for your use case of putting entities into a map: I 
notice that you're using the TimelineEntity instances directly as keys to maps. 
Wouldn't it be better to use their Identifier instances as keys instead? 
Identifier instances are easier and cheaper to construct and compare.
I think I used an inappropriate example here. I meant to say HashSet but not 

bq. We should make isValid() a proper javadoc hyperlink

bq. Since we're checking the entity type and the id, wouldn't it be sufficient 
to check whether the object is an instance of TimelineEntity?
I agree. Fixed all related ones. 

> add equals and hashCode to TimelineEntity and other classes in the data model
> -----------------------------------------------------------------------------
>                 Key: YARN-3836
>                 URL: https://issues.apache.org/jira/browse/YARN-3836
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: YARN-2928
>            Reporter: Sangjin Lee
>            Assignee: Li Lu
>         Attachments: YARN-3836-YARN-2928.001.patch, 
> YARN-3836-YARN-2928.002.patch
> Classes in the data model API (e.g. {{TimelineEntity}}, 
> {{TimelineEntity.Identifer}}, etc.) do not override {{equals()}} or 
> {{hashCode()}}. This can cause problems when these objects are used in a 
> collection such as a {{HashSet}}. We should implement these methods wherever 
> appropriate.

This message was sent by Atlassian JIRA

Reply via email to