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

Options/Attributes builders/build instances are no more "readable" #1080

Open
rmannibucau opened this issue Jan 24, 2022 · 5 comments
Open

Comments

@rmannibucau
Copy link
Contributor

Hi,

The main issue in recent version is the map access is deprecated but it is actually very useful to have a "template" instance (options/attributes) and copy+modify it for renders at needs.
Would be neat to get back something at least readable - best being to create an option builder from an options (resp attribute) instance as a copy.

@abelsromero
Copy link
Member

abelsromero commented Jan 29, 2022

Hard to recall all the conversation but we wanted to have typed cleaner methods that was more Java like and less like a simple Ruby wrapper.
You should still be able to keep "template" options/attributes as Maps on your application layers and then use iterate over attribute(String,Object) and option(String,Object) when converting. It will require extra code but would help decreasing coupling between arguments parsing/collection and conversion.

// Application layer
        Map<String, String> attributesTemplate = new HashMap<>();
        attributesTemplate.put("attribute-1", "value-1");
        attributesTemplate.put("attribute-2", "value-2");

        Map<String, String> optionsTemplate = new HashMap<>();
        attributesTemplate.put("option-1", "value-1");
        attributesTemplate.put("option-2", "value-2");

// Converter Helper
        final AttributesBuilder attributesBuilder = Attributes.builder();
        attributesTemplate.forEach((k, v) -> attributesBuilder.attribute(k, v));

        final OptionsBuilder optionsBuilder = Options.builder();
        optionsTemplate.forEach((k, v) -> optionsBuilder.option(k, v));

        final Options optionsToPass = optionsBuilder.attributes(attributesBuilder.build())
                .build();

@rmannibucau
Copy link
Contributor Author

I can see that but it is quite inconvenient and technically it is still a map so instance -> builder should be easily doable, no? Most important is an easy api after all IMHO.

@abelsromero
Copy link
Member

Working on removing the @Deprecated methods for v3.0.x I want to maintain the minimum for this.
So far I was thinking in maintaining the methods that return the underlying Map<String,Object>, but nothing else. When you said "map access" did you mean to maintain the constructors and setters with Map<String,Object>?

@rmannibucau
Copy link
Contributor Author

Only read part, factory is easy to workaround.

@ancore
Copy link

ancore commented Dec 15, 2023

Having access to the map (or contained attributes) is also very useful for unit testing of code that produces Attributes and Options.

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

No branches or pull requests

3 participants