dcapwell commented on code in PR #89:
URL: https://github.com/apache/cassandra-accord/pull/89#discussion_r1602283259


##########
accord-core/src/main/java/accord/local/CommandStores.java:
##########
@@ -590,6 +592,20 @@ public <O> AsyncChain<O> mapReduce(PreLoadContext context, 
Routables<?> keys, lo
         return chain == null ? AsyncChains.success(null) : chain;
     }
 
+    public <O> Map<Integer, O> map(PreLoadContext context, Function<? super 
SafeCommandStore, O> mapper)

Review Comment:
   this adds a blocking call, something we can not do.  This *must* be 
`AsyncChain<Map<Integer, O>>`.
   
   Also, not too much of a fan of `Map` rather than `List`...  the user 
function has access to the store so can include it in its output if needed... 
map just feels heavy weight and could reproduce the same results by using 
`accord.local.CommandStores#ids` with the list output (as they are the same 
order)
   
   
   I would recommend the following instead
   
   ```
   public <O> AsyncChain<List<O>> map(Function<? super SafeCommandStore, O> 
mapper)
       {
           return map(PreLoadContext.empty(), mapper);
       }
   
       public <O> AsyncChain<List<O>> map(PreLoadContext context, Function<? 
super SafeCommandStore, O> mapper)
       {
           ShardHolder[] shards = current.shards;
           List<AsyncChain<O>> results = new ArrayList<>(shards.length);
           for (ShardHolder shard : shards)
           {
               AsyncChain<O> next = shard.store.submit(context, mapper);
               results.add(next);
           }
           return AsyncChains.all(results);
       }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to