Should we standardize builder vs toBuilder in Datastore?

[ajkannan]

This is a minor issue, but Datastore uses a static "builder" in cases that we would have a non-static "toBuilder" method in other modules. For example, in Datastore's Key, there's public static Builder builder(Key copyFrom).

Do we care about standardizing this at some point before version 1.0.0 is released?

[eamonnmcmanus]

There's a distinction between a static method that returns a Builder, conventionally called builder(), and an instance method that returns a Builder initialized with the values from the current instance, conventionally called toBuilder(). That's the convention Guava follows, at least. The method you refer to should indeed be called builder(). Do other classes not follow this convention?

[ajkannan]

The classes do indeed follow the static builder/instance toBuilder convention. I think I was unclear in my original post. I was suggesting that we replace the static builder that takes a (for example) Key object and copies the values with an instance method toBuilder that doesn't require that parameter.

@aozarov tells me that he may have tried this a while back but ran into problems, perhaps related to the inheritance hierarchy of Key and IncompleteKey. I can experiment to see if it's possible/worth it to make this change.

[ajkannan]

Indeed, because the Builder of Key doesn't extend the Builder of IncompleteKey, both IncompleteKey and Key can't have a toBuilder() method (and the same goes for Entities). Making the Builder of Key extend the Builder of IncompleteKey is painful and not worth the effort for the gain. I'll close this issue.