Skip to content

Should we standardize builder vs toBuilder in Datastore? #628

Closed

Description

@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?

Activity

  1. added
    api: datastoreIssues related to the Datastore API.
    type: questionRequest for information or clarification. Not an issue.
    on Feb 6, 2016
  2. eamonnmcmanus commented on Feb 6, 2016

    @eamonnmcmanus
    Contributor

    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?

  3. ajkannan commented on Feb 6, 2016

    @ajkannan
    Author

    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.

  4. ajkannan commented on Feb 6, 2016

    @ajkannan
    Author

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api: datastoreIssues related to the Datastore API.type: questionRequest for information or clarification. Not an issue.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions