Pull to refresh

Comments 22

Но этот код неработоспособен, т.к. метод Get() накладывает ряд ограничений на тип, с которым работает:

  • Тип T должен быть неабстрактным и содержать открытый конструктор без параметров
  • Гетеры свойств не должны генерировать исключений

Так в чём проблема изначально сделать

public sealed class SomeClientOptions
{
    public string Login { get; set; } = "NoLoginProvided";
    public string CertificatePath { get; set; } = "NoCertificatePathProvided";
}

или другие, более осмысленные дефолтные значения?
Как не писать руками лишний код?
Использовать NullGuard.

Да просто string.Empty как дефолтное значение. Для коллекций — пустая коллекция, и так далее. По аналогии с value types, у которых есть значение по умолчанию.


Чуть больше кода, зато решение очевидное и надёжное. null! лучше всё-таки избегать.

Да просто string.Empty как дефолтное значение. Для коллекций — пустая коллекция, и так далее. По аналогии с value types, у которых есть значение по умолчанию.
Да, но тогда нужно будет при использовании проверять наличие корректного значения для настройки — иначе вместо NRE мы получим какую-нибудь другую ошибку, которую, возможно, будет сложнее отладить.

Так что использование корректного дефолтного значения, на мой взгляд — лучший вариант.
На самом деле тут есть два тонких момента:
1. IConfiguration при десериализации все равно запишет NULL в свойства, которых нет в appsettings.json так что в set все равно нужно вставить гард
2. Во всех местах, где нужно выполнять работу со свойствами придется добавлять проверку на дефолтное значение, что само по себе эквивалентно проверки на нулл. Т.к. если мы передадим «NotCertPathProvided» в метод, который работает с путями, то можем получить очень странное поведение. Ну и в добавок, эти дефолтные значения нужно вынести еще куда-то в константы
IConfiguration при десериализации все равно запишет NULL в свойства, которых нет в appsettings.json

Вообще-то не должен.

1. IConfiguration при десериализации все равно запишет NULL в свойства, которых нет в appsettings.json так что в set все равно нужно вставить гард
Эм, нет не запишет. Выше указали ссылку на исходный код, да и я использую такой подход на практике — не перезаписывает.
2. Во всех местах, где нужно выполнять работу со свойствами придется добавлять проверку на дефолтное значение, что само по себе эквивалентно проверки на нулл. Т.к. если мы передадим «NotCertPathProvided» в метод, который работает с путями, то можем получить очень странное поведение.
Вы повторяете мой комментарий, на который отвечаете. Буквально первый абзац. И во втором абзаце я и пишу, что вместо просто не-null значений стоит использовать корректные дефолтные значения — которые не приведут к ошибкам. В таком случае, всё будет работать.
Ну и в добавок, эти дефолтные значения нужно вынести еще куда-то в константы
Ну разумеется, я же не весь проект сюда скидываю…

Подождите, но почему не IValidateOptions, который ровно для этого придуман?


А, теперь понял.


Наивная попытка использовать встроенные инструменты
public void ConfigureServices(IServiceCollection services)
{
var options = Configuration.GetSection(nameof(SomeClientOptions)).Get<SomeClientOptions>();
services.AddSingleton(options);
}

Вы вообще не в курсе про options pattern?

А как этот Options Pattern тут будет работать? Он же там "внутрях" точно так же попытается сделать .Get<SomeClientOptions>() и упадёт, разве нет?

Он же там "внутрях" точно так же попытается сделать .Get<SomeClientOptions>() и упадёт, разве нет?

Нет. Во-первых, он не обязан делать внутри Get<T> (собственно, он его и не делает, но это не важно), ему можно что угодно запихнуть. Во-вторых, автор же предлагает в качестве второго варианта проверку в сеттере? Можно ее заменить на проверку в валидаторе, которая будет один раз для всех типов настроек.


UPD. Нет, про "второй вариант" я не прав. Но можно, тем не менее, встроить свою фабрику, которая для пользователей будет неотличима от обычного поведения options pattern.

Запихнуть-то туда можно что угодно, но при этом придётся решать всё те же самые проблемы.

Несомненно, где-то эти проблемы придется решать. Но лучше решать их по камушкам, чтобы вызывать наименьшее удивление у тех, кто этим кодом пользуется.


А еще я хочу сказать, что подход с валидатором позволяет выполнить вот это утверждение автора:


В качестве проверки на null я буду использовать string.IsNullOrEmpty()

С которым nullability не справляется.

Он же там "внутрях" точно так же попытается сделать .Get<SomeClientOptions>() и упадёт, разве нет?

Нет, не упадет. Меня это изначально-то удивляло, так что я проверил:


class Program
{
    static void Main(string[] args)
    {
        var configuration = new ConfigurationBuilder()
            .AddInMemoryCollection(new KeyValuePair<string, string>[]
            {
                new KeyValuePair<string, string>("CertificatePath", "somePath"),
            })
            .Build();

        var provider = new ServiceCollection()
            .Configure<SomeClientOptions>(configuration)
            .BuildServiceProvider();

        var options = provider.GetRequiredService<IOptions<SomeClientOptions>>().Value;

        Console.WriteLine($"Login is null: {options.Login == null}");
        Console.WriteLine($"CertificatePath: {options.CertificatePath}");
    }
}

public sealed class SomeClientOptions
{
    public string Login { get; set; } = null!;
    public string CertificatePath { get; set; }= null!;
}

Вывод:


Login is null: True
CertificatePath: somePath

Достаточно добавить валидатор.


(Собственно, чтобы Get не падал, достаточно конструктора без параметров.)

Какой вообще смысл в конструкции null!? Это же оксюморон и ломает всю логику nullable reference types.
Какой вообще смысл в конструкции null!?

Удовлетворить компилятор.


Не, повторюсь, можно написать свой биндер, который будет не через свойства работать, а через параметры конструктора. Просто придется больше кода написать.

Обратите внимание, что исходная ваша цитата, про наивную попытку использовать встроенные инструменты, относилась к другому коду. А именно, встроенные инструменты автор пытался использовать вот для этого кода:


    public sealed class SomeClientOptions
    {
        public string Login { get; set; }

        public string CertificatePath { get; set; }

        public SomeClientOptions(string login, string certificatePath)
        {
            Login = login;
            CertificatePath = certificatePath;
        }
    }

Замените класс и попробуйте ещё раз, если не верите.

Да нет, я верю, что с этим кодом не работает (именно из-за конструктора). Но вопрос же в том, как это починить с минимальным количеством кода (и WTF).

Кажется, эти условия, про минимальное количество кода и минимальное количество WTF, противоречат друг другу.


Потому что каждый null! — это WTF. А чтобы избавиться от WTF, нужно делать два класса, что автоматически делает количество кода неминимальным...

Кажется, эти условия, про минимальное количество кода и минимальное количество WTF, противоречат друг другу.

Угу, поэтому надо какой-то компромис выбрать.


Потому что каждый null! — это WTF.

Не уверен, что это WTF для того, кто этот экземпляр использует.


А чтобы избавиться от WTF, нужно делать два класса

Или свой собственный биндер.

Слушайте, IValidateOptions полезная штука, но не понятно, как он решает проблему nullability.
Все равно придется писать там код типа


public sealed class SomeClientOptions
{
    public string Login { get; set; } = null!;
}

Ну и плюс надо писать [Required] видимо, чтобы Microsoft.Extensions.Options.DataAnnotations срабатывал.
Или я еще чего-то не учитываю?

Все равно придется писать там код типа

Придется, да. Этот код вообще придется писать, если мы работаем с options pattern (кроме дефолтного null!), так что это само по себе не проблема.


Или я еще чего-то не учитываю?

Можно написать свой собственный валидатор на том же подходе, что у автора выше, чтобы не надо было даже [Required] писать. Получаем, собственно, тот минимум кода, к которому стремимся.

Дефолтный null! в основном раздражает.


А про валидатор — по логике DataAnnotations должен бы реагировать на nullable/не nullable, если они есть — это ведь тоже стандартные атрибуты. Надо бы проверить

Дефолтный null! в основном раздражает.

Он меня раздражает меньше, чем собственный костыль для инъекции настроек, минующий IOptions.

Sign up to leave a comment.

Articles