Skip to content

Creating an expandable enum type, converting StorageClass#1937

Merged
garrettjonesgoogle merged 3 commits intogoogleapis:masterfrom
garrettjonesgoogle:alt
Apr 18, 2017
Merged

Creating an expandable enum type, converting StorageClass#1937
garrettjonesgoogle merged 3 commits intogoogleapis:masterfrom
garrettjonesgoogle:alt

Conversation

@garrettjonesgoogle
Copy link
Member

@garrettjonesgoogle garrettjonesgoogle commented Apr 18, 2017

Design notes:

  • I made it so that converting an enum into a StorageEnumValue is not disruptive at a code level (e.g. it has valueOf and values, same as Java enums)
  • I am only converting one enum now, and after we agree on the design, I will do a follow-up PR to convert the rest.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 18, 2017
@garrettjonesgoogle
Copy link
Member Author

garrettjonesgoogle commented Apr 18, 2017

Updates #1832

shinfan
shinfan reviewed Apr 18, 2017
View reviewed changes
Copy link
Contributor

@shinfan shinfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The design looks reasonable to me. Just a few comments.

google-cloud-storage/src/main/java/com/google/cloud/storage/StorageClass.java
* Get the StorageClass for the given String constant, and throw an exception if the constant is
* not recognized.
*/
public static StorageClass valueOfStrict(String constant) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-core/src/main/java/com/google/cloud/StringEnumValue.java
public abstract class StringEnumValue {
private String constant;

@InternalApi

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-core/src/main/java/com/google/cloud/StringEnumType.java Outdated

public StringEnumType(String enumName, ApiFunction<String, EnumT> constructor) {
this.enumName = enumName;
this.constructor = constructor;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-core/src/main/java/com/google/cloud/StringEnumType.java Outdated

private final String enumName;
private final ApiFunction<String, EnumT> constructor;
private Map<String, EnumT> knownValues = new HashMap<>();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@michaelbausor
Copy link
Contributor

michaelbausor commented Apr 18, 2017

Design looks good to me too. Two points: (1) Is may be possible to push some functionality from StorageClass into StringEnumValue by making StringEnumValue parametrized recursively, e.g. class StringEnumValue<T extends StringEnumValue<T>>. This reduces the code in StorageClass, at the expense of more complexity. (2) Do we need tests for StringEnumValue and StringEnumType?

@garrettjonesgoogle
Copy link
Member Author

garrettjonesgoogle commented Apr 18, 2017

(1) Which functionality can be moved up? The only public things in StorageClass are static.
(2) It does make sense to make tests for StringEnumValue and StringEnumType - I'll add those in.

@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Changes Unknown when pulling 6db00d3 on garrettjonesgoogle:alt into ** on GoogleCloudPlatform:master**.

@michaelbausor
Copy link
Contributor

michaelbausor commented Apr 18, 2017

Re (1): you are right.

@garrettjonesgoogle
Copy link
Member Author

garrettjonesgoogle commented Apr 18, 2017

Added unit tests, PTAL

@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Changes Unknown when pulling d6f19ab on garrettjonesgoogle:alt into ** on GoogleCloudPlatform:master**.

@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Changes Unknown when pulling ded5357 on garrettjonesgoogle:alt into ** on GoogleCloudPlatform:master**.

@michaelbausor
Copy link
Contributor

michaelbausor commented Apr 18, 2017

LGTM

@garrettjonesgoogle garrettjonesgoogle merged commit cc8dc45 into googleapis:master Apr 18, 2017
chingor13 pushed a commit that referenced this pull request Jan 22, 2026
@cloud-java-bot
chore: Update generation configuration at Sat Aug 16 02:30:30 UTC 2025 (
2b7fed8 
#1937)

* chore: Update generation configuration at Sat Aug  9 02:31:56 UTC 2025

* chore: Update generation configuration at Sun Aug 10 02:43:05 UTC 2025

* chore: Update generation configuration at Mon Aug 11 02:41:17 UTC 2025

* chore: Update generation configuration at Tue Aug 12 02:32:03 UTC 2025

* chore: Update generation configuration at Wed Aug 13 02:32:41 UTC 2025

* chore: Update generation configuration at Thu Aug 14 02:34:29 UTC 2025

* chore: Update generation configuration at Fri Aug 15 02:34:47 UTC 2025

* chore: Update generation configuration at Sat Aug 16 02:30:30 UTC 2025

* chore: generate libraries at Sat Aug 16 02:31:01 UTC 2025
chingor13 pushed a commit that referenced this pull request Feb 20, 2026
@release-please @Neenu1995
chore(main): release 2.28.3-SNAPSHOT (#1937)
b7c5908 
* chore(main): release 2.28.3-SNAPSHOT

* chore: clirr updates

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Neenu1995 <neenushaji@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelbausor michaelbausor Awaiting requested review from michaelbausor

1 more reviewer

@shinfan shinfan shinfan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@garrettjonesgoogle @michaelbausor @coveralls @shinfan @googlebot