Home | About | Sematext search-lucene.com search-hadoop.com
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB
 Search Hadoop and all its subprojects:

Switch to Threaded View
Zookeeper >> mail # dev >> Why does multi-op check operation increment zxid?


Copy link to this message
-
Re: Why does multi-op check operation increment zxid?
to be honest it is lines like this

-               switch (hdr.getType()) {
-               case OpCode.createSession: {
+               switch (OpCode.fromInt(hdr.getType())) {
+               case createSession: {

and especially
-                        h.setType(ZooDefs.OpCode.setWatches);
+                        h.setType(OpCode.setWatches.getInt());

that ruin the enum for me. it's not that i don't like enums, it's just
that in this case we are using a serialization class that is
generating ints that sucks away a lot of the benefits. a field that
keeps switching between int and enum is just a bit weird and a little
confusing. i'd rather stick with consistency. we should, and do, use
enums if it is a field that is always used as a enum.

i actually started to write some of the things you are converting to
enums as enums originally, but i found the switching code just a
little too ugly, and i abandoned it.

i really don't feel strongly about this case, since i have thought
both ways about this particular field in the past. i do feel rather
strongly about a big patch for a rather non-issue. and your patch is
very big, and there is a bunch of other issues we really need to be
addressing. so, it's sucking up some valuable committer time.

having said that, if a committer does think it is a good idea and
wants to shepherd the patch, i certainly wouldn't stand in the way.

ben

On Mon, Oct 24, 2011 at 10:52 AM, Thomas Koch <[EMAIL PROTECTED]> wrote:
> Benjamin Reed:
>> i was thinking mostly in terms of the code itself. enums are great for
>> structures that aren't serialized, but when you do serialize it, you
>> end up having to force specific values into the enum and converting
>> back and forth during the serialization and deserialization. i find
>> that the enums, in this case, just shift the ugliness around.
>>
>> i'm also not sure about the performance implication, it should just be
>> the cost of autoboxing, but to be honest it's more about the code
>> readability to me.
>>
>> ben
> Hi Ben,
>
> I've updated the patch for ZK-1199 (make OpCode an enum). Please see yourself,
> whether you consider the resulting code to be uglier or not. Especially in the
> following classes I could remove a couple of lines and complexity:
>
> PrepRequestProcessor, CommitProcessor, FollowerRequestProcessor,
> ObserverRequestProcessor, ReadOnlyRequestProcessor
>
> If you're concerned about readability, we should trust Joshua Bloch. One of
> the advantages pointed out by him is type safety. The Request class has long
> zxid, long sessionId, long cxid, and int type. During refactoring I've had a
> hard time to not mix positions of the constructor parameters. The IDE can not
> help me to distinguish between different number types.
>
> Best regards,
>
> Thomas Koch, http://www.koch.ro
>
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB