zookeeper-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [zookeeper] szepet commented on issue #959: ZOOKEEPER-3402: Add multiRead operation
Date Sat, 15 Jun 2019 00:25:26 GMT
szepet commented on issue #959: ZOOKEEPER-3402: Add multiRead operation
URL: https://github.com/apache/zookeeper/pull/959#issuecomment-502317607
 
 
   Thank you all for sharing your ideas and suggestions!
   I think I've managed to address most of the comments and asked improvements.
   @stickyhipp Thank you for the benchmark! It is really neat to have measurements like these
and I am glad this universal API turned out as useful.
   @lvfangmin That is a really good (and hard) question where both solutions could work and
it rather depends on someone's taste which way to choose! I've answered your comment above
but quoting it here as well:
   > On one hand, I definitely agree with you that the name is confusing and could be improved.
Going into implementation details, I think it would be the best to create a `MultiOperationRecord`
base (super) class which could have `MultiReadRecord` and `MultiTransactionRecord` subclasses.
   > On the other hand, (besides resolving confusion caused by the name) I do not see how
it helps as to avoid any checks. It would require us to overload some function for Txn/Read
operations which would lead to more duplicated code. Another way is that we could have an
`instanceof` check at the beginning of these function instead of the `OpKind` checks but again,
this would result the some checks.
   > Since `Op,` `OpResult,` and `MultiResponse` is preserved as a single class (though
these could be extended into more subclasses as well like `ReadOp/TransactionOp`, `MultiTxnResponse/MultiReadResponse`)
to maintain the current workflow, I say it is better and more consistent to preserve it as
well and make decisions based on `OpKind.`
   > But of course, I am convincible on this topic so feel free to argue! :) What do you
think?
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message