I'm trying to setup generic classes within IOptions settings. Example:
public class MyClass<T> : IMyClass
{
public MyClass(IOption<MyOptions<T>> options)
{
}
}
To be able to dynamically use settings from appsettings.json. While the set of options is the same, they should be under different schemas, so that each type defines its set of options i.e.:
"MyOptionsForClass1":
{
"Option1" : 1,
"Option2" : 1
},
"MyOptionsForClass2":
{
"Option1" : 2,
"Option2" : 2
},
And the options classes should look like this:
public class MyOptions<T>
{
public const string ConfigSchemaName = "DefaultOptions"
public int Option1 {get; set;}
public int Option2 {get; set;}
}
public class OptionsForClass1 : MyOptions<MyClass1>
{
public new const string ConfigSchemaName = "MyOptionsForClass1"
public int Option1 {get; set;}
public int Option2 {get; set;}
}
And then in ConfigureServices to add something like:
.AddScoped<MyOptions<MyClass1>, OptionsForClass1>();
...
.AddOptions<MyOptions<MyClass1>>()
.Bind(configuration.GetSection(OptionsForClass1.ConfigSchemaName);
Is this how this should be done? Are there any other more clean approaches for this?
Thank you!
CodePudding user response:
You definitely should not do it this way.
Note the squiggly lines under Option1
and Option2
? It's telling you that you are hiding the members in the parent class, but you forgot to use the new
keyword.
But using new
is bad for hiding (shadowing) members. Try this code:
var ofc1 = new OptionsForClass1();
ofc1.Option1 = 42;
Console.WriteLine(ofc1.Option1);
var my = (MyOptions<MyClass1>)ofc1;
Console.WriteLine(my.Option1);
That outputs:
42
0
Note that I've only created one instance of OptionsForClass1
but when I cast to MyOptions<MyClass1>
I get a different value for Option1
.
Your approach kills the value of inheritance.
If you do want this kind of structure, your code should look like this:
public class MyOptions<T>
{
public virtual string ConfigSchemaName => "DefaultOptions";
public virtual int Option1 { get; set; }
public virtual int Option2 { get; set; }
}
public class OptionsForClass1 : MyOptions<MyClass1>
{
public override string ConfigSchemaName => "MyOptionsForClass1";
}
You could avoid a const string
by using an attribute.
Try this:
public class ConfigSchemaNameAttribute : Attribute
{
public string Name { get; init; }
public ConfigSchemaNameAttribute(string name)
{
this.Name = name;
}
}
[ConfigSchemaName("MyOptionsForClass1")]
public class OptionsForClass1 : MyOptions<MyClass1>
{
}
Then you can write this:
Console.WriteLine(
typeof(OptionsForClass1)
.GetCustomAttribute<ConfigSchemaNameAttribute>()
.Name);
That outputs:
MyOptionsForClass1
CodePudding user response:
Is this how this should be done?
It's definitely how it can be done. Should be done? That's pretty subjective, once the solution meets a certain standard of quality.
Your solution falls into the over-engineered category, where you have a very intricate solution to a problem that has simpler solutions. For example, you could have a simple dictionary of settings you inject in every object, and each object takes out what settings it wants. Very simple and efficient, both at run-time and while coding -- no boilerplate at all, one class for all cases!
But if you have a lot of options, maybe you need the over-engineered solution. There's definitely nothing wrong with it, besides it being over-engineered. Though I'd suggest separate settings files if you have such an incredible amount of settings that you need separate classes for all of them.