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

ASF GitHub Bot commented on NIFI-4504:
--------------------------------------

Github user devriesb commented on the issue:

    https://github.com/apache/nifi/pull/2284
  
    Koji,
    
    "Are all of these three methods, removeAndGet, removeByPatternAndGet and
    keySet required by the folks you know of?" - Yes.
    
    "atomicity is not that important in these method IMHO" - I think you'll
    find not everyone shares that opinion.
    
    Ultimately, removeAndGet behaves the same as the remove() method of the
    java Map interface, which is the behavior I (and I would suspect many
    others) would expect.  While I understand providing a "flavor" of this
    method that does not return the removed value to save network traffic, I
    would have preferred those be named differently.  However, since we're now
    locked into "remove" not returning the removed value, "removeAndGet" is
    likely as good as we're going to get.
    
    Brandon
    
    
    
    On Wed, Dec 27, 2017 at 7:59 PM Koji Kawamura <notificati...@github.com>
    wrote:
    
    > @mosermw <https://github.com/mosermw> Thank you for updating the PR, it's
    > much easier to review now. The code implemented at
    > DistributedMapCacheClientService and MapCacheServer looks good to me.
    > Travis error should be fine as the failure looks depending on execution
    > condition.
    >
    > However, let me ask one more time since adding methods is easy but hard to
    > remove afterwards for things like this. Are all of these three methods,
    > removeAndGet, removeByPatternAndGet and keySet required by the folks you
    > know of? I prefer to minimize the addition as those methods are only
    > supported by DistributedMapCacheClientService, which means those will not
    > be used by most developers who write Processors. Also, it confuses such
    > developers which to choose from remove and removeAndGet. Probably I
    > imagine these concerns are what made you struggle with implementing 
default
    > method to just throw UnsupportedOperationException.
    >
    > If there is no significant needs for removeAndGet and
    > removeByPatternAndGet, then I'd prefer omit these method from
    > DistributedMapCacheClient interface. As long as we support keySet, pretty
    > much everything can be done at the caller side. The only benefit to have
    > AndGet methods I am aware of is reducing the network traffic, and 
atomicity
    > (atomicity is not that important in these method IMHO). Do you think those
    > are more important than adding mostly unsupported methods into a common
    > interface?
    >
    > —
    > You are receiving this because you are subscribed to this thread.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/nifi/pull/2284#issuecomment-354207212>, or mute
    > the thread
    > 
<https://github.com/notifications/unsubscribe-auth/AB2qyBEO72vOhV9p-2HcFIUoAS3_UM9xks5tEufigaJpZM4Qmhur>
    > .
    >



> SimpleMapCache/PersistentMapCache: Add removeAndGet and removeByPatternAndGet
> -----------------------------------------------------------------------------
>
>                 Key: NIFI-4504
>                 URL: https://issues.apache.org/jira/browse/NIFI-4504
>             Project: Apache NiFi
>          Issue Type: Improvement
>    Affects Versions: 1.4.0
>            Reporter: Jon Kessler
>            Assignee: Michael Moser
>            Priority: Minor
>
> Typical map implementations return the value that was removed when performing 
> a remove. Because you couldn't update the existing remove methods without it 
> being a breaking change I suggest adding new versions of the remove and 
> removeByPattern methods that return the removed value(s).
> These changes should also be applied up the chain to any class that makes use 
> of these classes such as the MapCacheServer and 
> AtomicDistributedMapCacheClient.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to