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

New issue 451 by dmtes...@gmail.com: Make SingleFieldBuilder truly extendable.
http://code.google.com/p/protobuf/issues/detail?id=451

What steps will reproduce the problem?
Though SingleFieldBuilder allowed to be subclassed, it's really locked due to:
1. all fields are private w/o accessor methods.
2. there are no protected methods to alter its behavior.
3. many its methods access protobuf package-private classes/fields, so without proper hooks (via protected methods), even the complete source-reuse is not possible.

What is the expected output? What do you see instead?
Here are the alternatives in order of the increased openness:
1. (Best and Safest): Inside SingleFieldBuilder.getBuilder() call the protected BType getBuilder(MType Message, , BuilderParent parent) which returns a Builder for the passing message (the private message field). This way, a subclass can control how and Builder is returned. Then there is no need to open up any private fields. This case is very important/crucial for any kind of "universal" GeneratedMessage Builders, and also is safe. Here is the proposed code:

public BType getBuilder() {
  if (builder == null) {
    // builder.mergeFrom() on a fresh builder
    // does not create any sub-objects with independent clean/dirty states,
// therefore setting the builder itself to clean without actually calling
    // build() cannot break any invariants.
    builder = getBuilder(this.message, this);
    builder.markClean();
  }
  return builder;
}

protected BType getBuilder(MType message, BuilderParent parent) {
  builder = (BType) message.newBuilderForType(parent);
  builder.mergeFrom(message); // no-op if message is the default message
}

Thus, subclass can override getBuilder(MType message), for example, by using its own concrete BType Builder factory, allowing for any kind of MType message and the accommodating concrete BType builder. It is hugely important for performance reasons, providing for caching any broadly defined MType message and the more or less universal BType Builder which knows how to get itself based on on the message. Currently, it is required for the Message to know its builder, and can be extended only by making the unnecessary Message wrapper.

2. (Complete openness, less safe): Make all fields protected, or create and use throughout protected accessors methods.


What version of the product are you using? On what operating system?
protobuf 2.4.1 and 2.5rc1.

Please provide any additional information below.


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

Reply via email to