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

Feature request: Allow chained/fluent setters #128

Open
djarnis73 opened this issue Apr 27, 2020 · 9 comments
Open

Feature request: Allow chained/fluent setters #128

djarnis73 opened this issue Apr 27, 2020 · 9 comments

Comments

@djarnis73
Copy link

Hi

I tend to use chained setters quite a lot and was wondering if it would be difficult to allow them, either by default or through configuration.

I wouldn't mind cooking up a PR if i think it would be a good idea (and if it is not too complex).

I did do a search through the issues to see if this is a duplicate, but did not find anything.

Best regards Jens

@schuettec
Copy link
Contributor

schuettec commented Apr 28, 2020

Thank for your idea.
Can you give me a short example of what you want to do?
Just to get the use-case right.

My first guess would be, that the complexity for this feature might be high because if you want to chain setters, you have to have a valid instance of the source object available to invoke the setter on. The source object is generated later than the field-by-field mappings and currently there is no order defined on mappings.

@djarnis73
Copy link
Author

By chained setters I mean setter that return this like:

class B {
  int i;

  B setI(int i) {
    this.i = i;
    return this;
  }
}

So my proposal is to relax the pure java beans requirement for setters (either globally or it could be a mapper configuration).

@schuettec
Copy link
Contributor

schuettec commented Apr 29, 2020

Ah okay, so if your idea is to relax the validation of set-methods to support chained setters I would welcome a PR :)

Sounds very reasonable to support this.

@djarnis73
Copy link
Author

I did a bit of research and debugging and I don't think there is a simple way to achieve this, since the bean introspection does not allow for chained/fluent setters.

It used to back in java 1.6.

So to implement this, we will either have to use a different introspection engine (extra external dependency), commons-beantutils seems to have one or write our own.

I will dig a bit deeper into this. One option could also be to generate two artifacts remap & remap-fluent where the -fluent has the extra dependency. But that would be at the cost of extra complexity in both code and build setup.

@djarnis73 djarnis73 changed the title Feature request: Allow chained setters Feature request: Allow chained/fluent setters Apr 30, 2020
@djarnis73
Copy link
Author

I have made a proof of concept commit here: djarnis73@bcd99c8 please have a look and let me know if you like it.

@schuettec
Copy link
Contributor

Hi, very cool that you managed to do it!
That does not look that complicated. Good idea!

I would like to integrate your PR. I would check backwards compatibility with our code base an then we go for a release :)

@schuettec
Copy link
Contributor

Ah! One more thing. ReMap has an API for testing. Can you verify with a test, that the Assert API works well with your extension?

@djarnis73
Copy link
Author

How time flies, looks like I got distracted and never followed up on this. Are you still interested in getting this in? Then I will brush if off and submit a PR.

@schuettec
Copy link
Contributor

Hey it's been a while :) I think it's a nice idea to support fluent setters. As you mentioned above, keep in mind, that you need another bean introspection service. Feel free to build a PR. Then we can figure out how to integrate this into remap :)

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

2 participants