Quantcast

[infinispan-dev] to be a command, or not to be a command, that is the question

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[infinispan-dev] to be a command, or not to be a command, that is the question

Katia Aresti
Hi all,

As you might know I'm working since my arrival, among other things, on ISPN-5728 Jira [1], where the idea is to override the default ConcurrentMap methods that are missing in CacheImpl (merge, replaceAll, compute ... )

I've created a pull-request [2] for compute, computeIfAbsent and computeIfPresent methods, creating two new commands. By the way, I did the same thing for the merge method in a branch that I haven't pull requested yet.

There is an opposite view between Radim and Will concerning the implementation of these methods. To make it short :
In one side Will considers compute/merge best implementation should be as a new Command (so what is already done)
In the other side, Radim considers adding another command is not necessary as we could simple implement these methods using ReadWriteKeyCommand

The detailed discussion and arguments of both sides is on GitHub [2]

Before moving forward and making any choice by myself, I would like to hear your opinions. For the record, it doesn't bother me redoing everything if most people think like Radim because working on commands has helped me to learn and understand more about infinispan internals, so this hasn't been a waste of time for me.

Katia



_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] to be a command, or not to be a command, that is the question

Katia Aresti
Hi all

Well, nobody spoke, so I consider that everybody agrees that I can take a decision like a big girl by myself ! :) 

I'm going to add 3 new commands, for merge, compute&computeIfPresent and computeIfAbsent. So I won't use the actual existing commands for the implementation : ReadWriteKeyCommand and ReadWriteKeyValueCommand even if I'm a DRY person and I love reusing code, I'm a KISS person too.

I tested the implementation using these functional commands and IMHO :
- merge and compute methods worth their own commands, they are very useful and we might want to adjust/optimize them individually 
- there are some technical issues related to the TypeConverterDelegatingAdvancedCache that makes me modify these existing functional commands with some hacky code that, for me, should be kept in commands like merge or compute with the correct documentation. They don't belong to a generic command.
- Functional API is experimental right now. It might be non experimental in the near future, but we might decide to move to another thing. The 3 commands are already "coded" in my branches (not everything reviewed yet but soon). If one day we decide to change/simplify or we find a nice way to get rid of commands with a more generic one, removing and simplifying should be less painful than adding commands for these methods.

That's all !

Cheers

Katia



On Wed, Apr 12, 2017 at 12:11 PM, Katia Aresti <[hidden email]> wrote:
Hi all,

As you might know I'm working since my arrival, among other things, on ISPN-5728 Jira [1], where the idea is to override the default ConcurrentMap methods that are missing in CacheImpl (merge, replaceAll, compute ... )

I've created a pull-request [2] for compute, computeIfAbsent and computeIfPresent methods, creating two new commands. By the way, I did the same thing for the merge method in a branch that I haven't pull requested yet.

There is an opposite view between Radim and Will concerning the implementation of these methods. To make it short :
In one side Will considers compute/merge best implementation should be as a new Command (so what is already done)
In the other side, Radim considers adding another command is not necessary as we could simple implement these methods using ReadWriteKeyCommand

The detailed discussion and arguments of both sides is on GitHub [2]

Before moving forward and making any choice by myself, I would like to hear your opinions. For the record, it doesn't bother me redoing everything if most people think like Radim because working on commands has helped me to learn and understand more about infinispan internals, so this hasn't been a waste of time for me.

Katia




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Loading...