-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add extension method to IHttpClient that doesn't require cancellation token #382
Comments
@haacked Doh - that one slipped through the cracks 😮 My intention was to provide an overload. Any reason why you prefer extension methods? Either is fine with me. |
If it's an interface that's intended for others to implement, I tend to like minimalist interfaces implementations. The implementer only has to implement a single method and gets the extensions for free. Whereas if the interface has two methods, but one trivially calls the other, why make the implementer bother with implementing both? And worse, why give them the possibility of doing it wrong? That's why as a general principle of API design, I tend to not like overloads on interfaces and prefer extension methods. However, if the interface is solely for our testing purposes, I tend to like having the methods on the interface. It makes mocking it easier. :) However, if you write manual mocks, it actually makes it harder! |
Yep, we should always favor library consumers, which most of the time means extensions methods in a case like this. But keep in mind we also want to make TDD easy for our library consumers as well, so if it's an interface we expect consumers to stub, we should consider making it part of the impl. (This case doesn't seem like a thing a typical consumer would stub.) |
And in this case, stubbing the one method is pretty easy. :) |
I'm convinced - Extension method it is 👍 |
It looks like in 98e1556 we added a method to
IHttpClient
to pass in a cancellation token. But in a subsequent request, we took out the method that doesn't need the token.That's fine, but it might be useful to have an extension method on the interface that doesn't require a cancellation token.
/cc @ammeep
The text was updated successfully, but these errors were encountered: