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

Support building of immutable objects #12

Open
dburriss opened this issue Feb 25, 2018 · 6 comments
Open

Support building of immutable objects #12

dburriss opened this issue Feb 25, 2018 · 6 comments
Labels
enhancement planned issue is planned for a future release

Comments

@dburriss
Copy link

I am working on a PR to support a FluentImmutableBuilder class.

I am a bit unsure about the behavior when it is maybe not the best choice.

When could use FluentBuilder
When the FluentBuilder encounters no parameterless constructor it throws an error and tells the user to override the NewInstance method. There is some symmetry in then throwing if there is no constructor that takes arguments.
So should we throw a descriptive error message here?

New type
Maybe it would be better to add this as a configuration on FluentBuilder. Like .UseConstructorWithMostArguments() and UseConstructorOf(Type[])
Do we indeed need a new type or should it be added to existing class?

Property behavior
If new type, should it try set properties or just use constructor?

@dburriss
Copy link
Author

@nrjohnstone Thoughts?

@nrjohnstone
Copy link
Owner

nrjohnstone commented Feb 25, 2018

Sounds good, we are thinking that these methods would be in the ctor of the builder correct? like

public FooBuilder() {
    this.UseConstructorWithMostArguments();
}

Also I prefer moving away from the FluentBuilder class as it has a lot of baggage with the static wide configuration of "random" test data, which in most cases I've always only wanted a small % of the time. I don't really see any need to "inherit" from anything as I can't think of good reasons why my builders for test data would need PM

@dburriss
Copy link
Author

FooCtor is the builder right?

@dburriss
Copy link
Author

If so, yes. So you would like to go with changes to FluentBuilder to implement this rather than a new FluentImmutableBuilder?

@nrjohnstone
Copy link
Owner

I think a new FluentImmutableBuilder is the cleanest... replacing a FluentBuilder with the new FluentImmutableBuilder will be a bunch of work anyway if SetProperty has been used liberally and I think the way it will be used and it's rules will be completely different, so let's go with a new class to contain all of that

@dburriss
Copy link
Author

Ok will do. I am going to go with it not even trying to set properties then. Users can still use NewInstance override on FluentBuilder if they want to go that route.
So I think along that line it should throw if the class only has a parameterless constructor.

@nrjohnstone nrjohnstone added enhancement planned issue is planned for a future release labels Mar 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement planned issue is planned for a future release
Projects
None yet
Development

No branches or pull requests

2 participants