Skip to content

Added helper methods for spring #494

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

Merged
merged 1 commit into from
Sep 6, 2017
Merged

Added helper methods for spring #494

merged 1 commit into from
Sep 6, 2017

Conversation

garethahealy
Copy link
Collaborator

Added helper methods to the builder to make it easier for the end user when configuring dozer.

CC @orange-buffalo ; any views?

@orange-buffalo
Copy link
Contributor

@garethahealy, looks good to me, just one thought here. I would keep builder methods which accept single object instead of deprecating them. Imo, code like this:

    ...
    .withCustomConverter(MyConverterBuilder.create()
            .withThis(..)
            .withThat(..)
            .build())
    .withCustomConverter(AnotherConverter.ofSomething(..))
    .withCustomConverter(new SimpleCoverter())
    ...

is more expressive than

    ...
    .withCustomConverters(MyConverterBuilder.create()
            .withThis(..)
            .withThat(..)
            .build(),
            AnotherConverter.ofSomething(..),
            new SimpleCoverter())
    ...

We could call withCustomConverters with single argument multiple times, but then it looses its semantics.
Of course, this is a matter of personal preferences, do not insist that it is the only and the best way :)

@garethahealy
Copy link
Collaborator Author

@orange-buffalo ; removed the deprecated as requested. I am easy either way, so left them.

@garethahealy garethahealy merged commit 35b9ec9 into DozerMapper:master Sep 6, 2017
@orange-buffalo
Copy link
Contributor

@garethahealy thanks!

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.

2 participants