-
Notifications
You must be signed in to change notification settings - Fork 19
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
RFQ - Weigh in on whether the implicit operator (negating need for .Build()) is confusing or useful #60
Comments
Hey @cottsak. Thanks for the comment. The implicit operator is a bit controversial and Rob and I debated it a lot when I implemented it. There's a trade off between the convenience you mention and the desire to be quite explicit. I think it's clearly documented, which hopefully achieves the right balance. Basically, if you use var to define your variable you have to call Build, otherwise how does the compiler know what to create? And, if you want to use the implicit creation you have to explicitly declare the variable. But, given this particular scenario, I'm not quite sure how using the same antUser twice is working. If you are declaring it twice then I would expect two different instances with different Id values, but if you are declaring one variable and using it twice you'd obviously expect the same values... Do you have an example of the whole test? |
I can see how that would cause a problem, but I'm happy enough that it's discoverable when you realise that All of the code samples include an explciit I'm happy enough with that. |
I find the implicit operator misleading. It's there and so folks will use it. The presence of the operator "reveals" a contract to the user (aided by productivity tools like R#, etc that go and find these things more proactively) whether the examples always use Speaking to "documentation" @mwhelan, as I highlighted in this other issue, folks aren't going to go and read doco in the first instance and they may not even resort to the doco if something doesn't behave the way they expect. UX (which applies as much to a package/lib as much as a UI form) is predicated on conventions (among other things). Those conventions are expectations that users have in advance and implicitly apply to your interface. In this case, the explicit requirement to In terms of ROI, how did you guys justify adding the implicit operator in your "debating" @robdmoore @mwhelan? I like these tools and I want to promote them wherever I go provided it fits with my philosophy and I can advertise to other devs how easy they are. If I can understand your thinking then perhaps I can be more forgiving to these kinds of peculiarities. |
I personally don't use the implicit operator at all because I prefer the explicit .Build call and I also like One thing I will say though is that you should keep in mind that you are only but a single user Matt, other people's expectations may not match yours. I don't want to discount your opinion though since it's also possible that the majority of user's match your opinion. I'd prefer if you tone down your language though mate - this is an OSS library, not something we get paid for so calling decisions that have been made "smelly" and "peculiarities" rather than giving benefit of the doubt is a little "unsettling" to use your phrase from the other thread... If we have a few different users that also feel the implicit operator is confusing I don't object to taking it out, but I'd like to see a reasonable discussion about both sides and if we did it we'd obviously have to bump the major version since it's a breaking change. |
@robdmoore I'm a passionate guy as you know Rob and because I love this tool I can get direct when it fails to keep me perfectly satisfied. I also have a very narrow view of the world when it comes to writing great software and possibly because I know you a little Rob, I hold you (and by extension this package) to a higher standard than others. Perhaps I've done the wrong thing. But you're right, I am only one user. Thanks for engaging me. |
I'm building a user like
var antUser = new UserBuilder().WithUniqueName("ant");
and its failing a test. When I add the explicit build egvar antUser = new UserBuilder().WithUniqueName("ant").Build();
the test passes.What I think is happening is a combination of two things:
Id
field, andBuildObject()
internallyThe failure is due to the same
antUser
being used twice in the test setup and I'm expecting the identity of said User to be the same with both usages of the instance, only it's not! TheId
is different every time the implicit cast happens.This is a pretty big deal and I think most devs would expect the identity/equality to behave the same way I am.
The implicit operator is misleading in that it allows the dev to think they don't really need to call
.Build()
explicitly (yay! saved banging a few keys) and so they don't. It's not obvious that the fix is to call.Build()
. And then when you figure it out it seems smelly that you need to be aware of this peculiar detail.Thoughts?
The text was updated successfully, but these errors were encountered: