[ 
https://issues.apache.org/jira/browse/PROTON-964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14653523#comment-14653523
 ] 

ASF GitHub Bot commented on PROTON-964:
---------------------------------------

Github user gemmellr commented on a diff in the pull request:

    https://github.com/apache/qpid-proton/pull/48#discussion_r36180425
  
    --- Diff: 
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/CollectorImpl.java ---
    @@ -57,9 +58,13 @@ public void pop()
             }
         }
     
    -    public EventImpl put(Event.Type type, Object context)
    +    public EventImpl put(EventType type, Object context)
         {
    -        if (tail != null && tail.getType() == type &&
    +        if (type == null)
    +            throw new IllegalArgumentException("Type cannot be null");
    +        if (!type.isValid())
    +            throw new IllegalArgumentException("Cannot put events of type 
" + type);
    +        if (tail != null && tail.getEventType() == type &&
    --- End diff --
    
    I believe it is coaslescing 'duplicate' events.
    
    Your explanation of the changes you did make here reinforces why I think it 
should probably be changed to equals() now. Before your changes, it wasnt 
possible for the == check here to do the wrong thing. Following these changes 
in future it will be possible (even if not that likely) for this to do the 
wrong thing and it wont necessarily be the easiest thing to find then, whereas 
it is just now.


> Proton-J extensible event types
> -------------------------------
>
>                 Key: PROTON-964
>                 URL: https://issues.apache.org/jira/browse/PROTON-964
>             Project: Qpid Proton
>          Issue Type: Improvement
>          Components: proton-j
>    Affects Versions: 0.10
>            Reporter: Bozo Dragojevic
>            Assignee: Bozo Dragojevic
>             Fix For: 0.11
>
>
> Event.Type is an enum which makes it impossible to extend.
> Introduce a separate interface EventType and make Event.Type implement it.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to