Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Decoder that accepts custom TypeConverters #702

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Conversation

kilink
Copy link
Member

@kilink kilink commented Feb 13, 2024

Add a new Decoder, CustomDecoder, which allows users to easily specify custom TypeConverter implementations. CustomDecoder exposes a single static factory method, create, which accepts a Collection of TypeConverter.Factory instances.

As part of adding this feature, refactor DefaultDecoder in a re-usable base class, AbstractRegistryDecoder, which CustomDecoder also extends from.

}
return Optional.empty();
};
DefaultDecoder customDecoder = DefaultDecoder.newCustomDecoder(Collections.singletonList(customTypeConverterFactory));
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming here is ... unfortunate. Do you feel like it would be worthwhile to rename the classes a bit? Maybe move the existing code to a CustomizableDecoder that has your static factory and making DefaultDecoder be a sublcass of it that calls the constructor with an empty factory list?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would probably just create a base class that they both share in that case, AbstractDecoder / AbstractRegistryDecoder, or something like that.

Add a new Decoder implementation, CustomDecoder, which allows users to specify custom
type converters via its static factory method, create. To avoid duplicate code, introduce
a new package private base class, AbstractRegistryDecoder, which the existing DefaultDecoder
and CustomDecoder both extend. CustomDecoder will add the default factories used by
DefaultDecoder as well, placing them after the custom ones so that they may be overridden
and are only used as fallbacks.
@kilink kilink changed the title Allow specifying custom converters in DefaultDecoder Add a Decoder that accepts custom TypeConverters Feb 14, 2024
@akang31 akang31 merged commit f8de50c into 2.x Feb 14, 2024
10 checks passed
@rgallardo-netflix rgallardo-netflix deleted the custom-converters branch May 22, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants