[ https://issues.apache.org/jira/browse/PROTON-964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14661879#comment-14661879 ]
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_r36522483 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/engine/impl/CollectorImpl.java --- @@ -57,9 +58,16 @@ 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); + } + // XXX: @gemmellr points out that type equality is wrong for event de-duplication. What about context equality? --- End diff -- May now be wrong rather than definitely is wrong was my point, its now ambiguous due to the change to an interface for the event type...though I do wish I had just never bothered making the comment :) Context is probably more interesting, in that as it has been used it was to tie a particular proton object to a particular non-proton object for reference, so I can see there may be cases where you actually wouldnt want to de-dup for equivalent contexts and so using equals() instead of == could actually be wrong for the context. > 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)