Avoid declaring constructors with parameters if the properties of the type can be obtained from configuration settings.
When upgrading to the latest version of the LLama source, I can no longer get the LlamaSharpConfig from the configuration setting like so:
var llamaSharpConfig = GetConfigSettings<LLamaSharpConfigExt>(configuration, "LlamaSharpConfig");
private static T GetConfigSettings<T>(IConfiguration configuration, string key) where T : class
{
var value = (configuration.GetSection(key)?.Get<T>()) ?? throw new ArgumentNullException(nameof(configuration), $"Can't find {key} section of type {typeof(T).Name} in configuration");
return value;
}
... because of the nondefault constructor that was introduced in LlamaSharpConfig:
/// <summary>
/// Initializes a new instance of the <see cref="LLamaSharpConfig"/> class.
/// </summary>
/// <param name="modelPath">The path to the model file.</param>
public LLamaSharpConfig(string modelPath)
{
ModelPath = modelPath;
}
The workaround is simple: just derive from it:
/// <summary>
/// We need this workaround because of nondefault constructor in <see cref="LLamaSharpConfig"/> doesn't allow this type to be read from an appsettings file.
/// </summary>
private sealed class LLamaSharpConfigExt : LLamaSharpConfig
{
public LLamaSharpConfigExt()
: base(string.Empty)
{
}
}
... and use this LLamaSharpConfigExt instead to read from the configuration settings.
That's alright with me, but perhaps we could:
- either consider avoiding adding constructors with parameters on types that can be read from configuration settings
- or consider defining separate configuration setting types
- or just do nothing at all since it's up to the library users to decide what is to be put in the configuration.
The problem here is that ModelPath is a non nullable string, that means that after constructing the LlamaSharpConfig object ModelPath must not be null.
That's going to need a constructor of some kind. Or marking the property as required, but I think that would have the same issue with deserialisation?
I understand. Personally I use IValidateOptions<TOptions> or IValidatableObject in those cases, to separate the definition from its validation.