Status: New
Owner: liuj...@google.com
Labels: Type-Defect Priority-Medium

New issue 460 by dav.co...@gmail.com: Add isDefault() to Java Message interface as a fast means to verify whether it is a default instance
http://code.google.com/p/protobuf/issues/detail?id=460

What steps will reproduce the problem?
1. The only way currently to check whether a message is the default is to use (1) message == message.getDefaultInstanceForType(), or worse, via (2) Message#equals(). The (1) works okay for a concrete GeneratedMessage because it is a singleton (implementation detail), however, in DynamicMessage (1) won't work (not a singleton), and equals() is relatively expensive.

What is the expected output? What do you see instead?
message == message.getDefaultInstanceForType() doesn't work for DynamicMessage, and very implementation dependent. On the other hand, the equals() is relatively expensive as a means for a simple isDefault() check.

The proposed "boolean isDefault()" method on Message (possibly deeper, in the MessageLite and/or MessageBuilder) interface strictly defines this capability, which is VERY easy to implement in concrete classes, and also in DynamicMessage and the likes (the "empty" message should return isDefault()==true).

Alternatively, "boolean isDefault()" could be declared as an abstract method of AbstractMessage. This would avoid messing with the base interfaces, won't break any external code at the expense of implicit understanding that this method should be available via appropriate casting, for example, ((AbstractMessage) message).isDefault() for instances of AbstractMessage, and ((T) message).isDefault() for any non-subclass of AbstractMessage.

Obviously, the former alternative is more elegant and logically proper, but will break all non-subclasses of AbstractMessage. To mitigate this a bit, I propose:

1. Implement a fail-safe AbstractMessage#isDefault():

@Override
public boolean isDefault() {
  return this.equals(getDefaultInstanceForType());
}

2. Implement GeneratedMessage#isDefault():

@Override
public boolean isDefault() {
  return this == getDefaultInstanceForType();
}

2. Implement DynamicMessage#isDefault():

@Override
public boolean isDefault() {
  return (fields == FieldSet.<FieldDescriptor>emptySet())
      && (unknownFields == UnknownFieldSet.getDefaultInstance());
}

3. All GeneratedMessage and DynamicMessage will work out of the box efficiently, and only direct AbstractMessage subclasses will have slow performance of isDefault(), unless they override it.

What version of the product are you using? On what operating system?
protobuf 1.4.1, 1.5RC1 with Java 1.5 - 1.7 on Windows and Linux

Please provide any additional information below.

In Java 1.5RC1 the Message interface already enhanced with getParserForType(), so this is the good time to add the proposed isDefault() method and make small changes described above.

Thanks in advance,
Daid

--
You received this message because you are subscribed to the Google Groups "Protocol 
Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To post to this group, send email to protobuf@googlegroups.com.
Visit this group at http://groups.google.com/group/protobuf?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to