I've developing a .NET Core library meant to simplify the configuration of the authentication within our SSO system. It will expose two methods to be called in the Program.cs
(or Startup.cs
) of ASP.NET Core web apps:
public static AuthenticationBuilder AddFederatedAuthentication (this IServiceCollection services)
public static IApplicationBuilder UseFederatedAuthentication (this IApplicationBuilder app)
I was taking inspiration of similar packages, like AddGoogle and I found something that seems weird from a API design point of view: The connection with Google requires a client id and a client secret. Those options can be configured in an overload of the AddGoogle
method, but they are not enforced in the method signature, so it's easy to simply do services.AddGoogle()
and have a misconfigured app.
The same design is followed in similar libraries, like AddFacebook.
Wouldn't it be better to add required options as parameters to the method signature, to guide the usage of these libraries?
1 Answer 1
I share your frustrations with this design pattern. I guess the reasoning is that it makes everything "easy" for a default setup.
I think what happens is the developers take a section of their DI/Framework setup
services.AddTransient(x);
var endpoint = configuration.GetSection(y)
services.AddSingleton(new myThing());
and just copy it into an extension method. What's worse is they will often make the lower level setup constructors internal
so you cant even call them yourself if you need to do a more complex setup, or use a different DI framework etc.
Would a more complex builder pattern, or overloads on the "simple" method be the best approach? Its arguable.
What should be supported though is the ability to make derived classes, call constructors and use these classes in normal OOP ways rather than locking them down to a single approved usage.
Then if you don't like the provided helper classes, you are free to make your own.
-
The problem with making classes extensible is that they can be extended in ways that the original author did not anticipate, and then those ways often have to be supported. "Classic OOP" supports the notion that classes (from a framework, library or API) should only be interacted with through their public interface; deriving from another class assumes knowledge of and access to the base class's internal members.Robert Harvey– Robert Harvey09/05/2023 11:16:43Commented Sep 5, 2023 at 11:16
-
1This is why folks like Eric Lippert often advocate for
sealed
classes as a default. Classes should only be "unsealed" when they are specifically designed to be inherited from.Robert Harvey– Robert Harvey09/05/2023 11:19:52Commented Sep 5, 2023 at 11:19 -
1Sure, it makes things easier for the library developer, but the library is less functional and the classes 'closed for extension'. Remember in the extremes of this case we are talking about tying your libraries to a specific DI framework and Config file readerEwan– Ewan09/05/2023 12:38:24Commented Sep 5, 2023 at 12:38
-
@Ewan You're not wrong about "closed for extension", but Robert's point is more that no one ever asked if it was intended to be extended this way (because classes are by default extensible, it's usually not an explicitly made decision). Inheritance can open the door too much, whereas composition ensures that the library's original class can still only be used the way it was intended to be. A derived class is more liable to break things or shoot itself in the foot, it might need more concrete knowledge of the base class' implementation, etc.Flater– Flater09/05/2023 22:42:18Commented Sep 5, 2023 at 22:42
-
its a fair argument, but an academic one. In practice Its a basic ask to be able to instantiate an object without being forced to use DI, or to pass in a connection string to a constructor rather than have it read from config. It's expected that libraries have a degree of customisation options available, such as exposing event handers or being able to pass in alternate implementations of dependencies, Its expected that they should implement a mockable interface for tests. This pattern often prevents these thingsEwan– Ewan09/06/2023 13:28:22Commented Sep 6, 2023 at 13:28
Explore related questions
See similar questions with these tags.