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

Stop passing IFakeObjectCallRule to OnAfterCallIntercepted in 5.0.0 #1494

Closed
thomaslevesque opened this issue Nov 16, 2018 · 8 comments
Closed
Assignees
Milestone

Comments

@thomaslevesque
Copy link
Member

In #1491 (comment), @blairconrad said:

As I hint at, this hack wouldn't be necessary if IFakeObjectCallRule didn't expose Apply. But it needs to in order for people to create their own rules.

But I don't think that IInterceptionListener.OnAfterCallIntercepted needs to be passed an object that has an Apply method; there's no need for OnAfterCallIntercepted to call Apply.
I don't even know why it would need to call IsApplicable or NumberOfTimesToCall. Shall we explore changing this in a future major version? We could supply OnAfterCallIntercepted with a different interface that's a restricted version of IFakeObjectCallRule, or even remove the IFakeObjectCallRule parameter.

@thomaslevesque replied:

Seems like a good idea. I don't see any value in having the call rule available in OnAfterCallIntercepted, except for information purposes. Maybe we could just pass a string describing the rule, or just the type of the rule.

@blairconrad blairconrad changed the title Stop passing IFakeObjectCallRule to OnAfterCallIntercepted Stop passing IFakeObjectCallRule to OnAfterCallIntercepted in 5.0.0 Nov 29, 2018
@blairconrad
Copy link
Member

@thomaslevesque, I think passing the type is a good idea.

@thomaslevesque thomaslevesque added this to the vNext milestone Nov 30, 2018
@thomaslevesque
Copy link
Member Author

@thomaslevesque, I think passing the type is a good idea.

Thinking about this again, I wonder if it's the right thing to do. The type isn't always enough to identify a rule. For instance, the user could create a custom rule, and add several different instances of that rule. Maybe we could pass some kind of "rule descriptor" rather than just the type? Or this:

We could supply OnAfterCallIntercepted with a different interface that's a restricted version of IFakeObjectCallRule

Of course, it's more work for little benefit, since nobody seems to use custom rules...

@blairconrad
Copy link
Member

The type isn't always enough to identify a rule. For instance, the user could create a custom rule, and add several different instances of that rule.

It's true. Of course, if it's all their code, they should be able to figure out which one it was. On the other hand, if it's one of our rules, what's the benefit of them finding out which one it was?

Really, I'm not sure why anyone wants to know which rule matched.

We could supply OnAfterCallIntercepted with a different interface that's a restricted version of IFakeObjectCallRule

Sure, that would be fine.

Of course, it's more work for little benefit, since nobody seems to use custom rules...

… and probably even fewer the interceptors. But if you'd like to (or have me) make a new interface for this purpose, I do not mind.

@thomaslevesque
Copy link
Member Author

Bah, forget it, it's not worth the trouble.

@blairconrad
Copy link
Member

So you want to stick with type? We could remove the rule-ish parameter altogether.

@thomaslevesque
Copy link
Member Author

I was just wondering about that. The type isn't enough information, but we don't want to go out of our way to provide more. So yes, maybe we should just remove the parameter.

@thomaslevesque
Copy link
Member Author

I'll send a PR with both options.

@thomaslevesque
Copy link
Member Author

This change has been released in FakeItEasy 5.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants