You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Should we standardize builder vs toBuilder in Datastore? #628
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?
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?
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.
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.
Activity
eamonnmcmanus commentedon Feb 6, 2016
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 calledtoBuilder(). That's the convention Guava follows, at least. The method you refer to should indeed be calledbuilder(). Do other classes not follow this convention?ajkannan commentedon Feb 6, 2016
The classes do indeed follow the static builder/instance
toBuilderconvention. 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 methodtoBuilderthat 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 commentedon Feb 6, 2016
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.
chore: remove readme autosynth config (googleapis#628)
deps: update dependency com.google.cloud:google-cloud-shared-dependen…
chore(main): release 2.5.2 (#630)
deps: update google.core.version to v2.5.10 (#628)
deps: update dependency com.google.cloud:google-cloud-shared-dependen…
chore: release 0.122.7 (#629)
deps: update guava patch (#628)
chore: release 0.25.3 (#630)
deps: update guava patch (#628)
chore: release 0.25.3 (#630)
chore: change branch master to main in github configurations (googlea…
deps: update google-cloud-* libraries to v2 (#639)
deps: update google-cloud-* libraries to v2 (#639)
chore: release 1.112.3 (#628)
chore: release 1.112.3 (#628)