Return null in TryXyz methods - Библиотека CodeJam

Камрад @NN__ поднял в реквесте вопрос про возврат null из TryXxx-методов.

Предложение — заменить (все?) такие методы на FW-style bool TryGet(..., out var result). Если неправильно понял или криво сформулировал — пиши, поправлю.

Чтобы не мучать мой английский и привлечь больше участников дублирую тему тут.

Моя точка зрения: я за использование текущего подхода по следующим причинам:

1. Работает. На самом деле это основная и единственная причина.
CJ чисто исторически сложился как сборник штук, которые работают и проверены практикой.

Use cases:

    // async
    var data1 = await cache.TryGetAsync(someKey1); // no way out arg can be here

    // elvis-op & nullable chains:
    var workers1 = consoleArgs.TryGetValue(_workersCountArg)
        ?.ToInt32Invariant()
        ?? 1
    // vs
    var workers2 = consoleArgs.TryGetValue(_workersCountArg, out var result)
        ? result.ToInt32Invariant() ?? 1
        : 1;

    // No way to use together with optional args.
    // By convention the out var result should be the last argument.
    [CanBeNull]
    public LambdaExpression TryGetConvertExpression(
        [NotNull] Type from,
        [NotNull] Type to,
        bool checkNull = true,
        bool createDefault = true)
    {
        Code.NotNull(from, nameof(from));
        Code.NotNull(to,   nameof(to));

        var li = GetConverter(from, to, createDefault);
        return li == null ? null : (LambdaExpression)ReduceDefaultValue(checkNull ? li.CheckNullLambda : li.Lambda);
    }

    // using
    using (var stream = resourceKey.TryGetResourceStream()) // no way out arg can be here
    {
        ...
    }

2. Нет реальных причин для изменений. "Рекомендуется стандартами" — это не причина.
Да, guidelines — это сильный аргумент при выборе из нескольких равноценных решений.
В частности, мы всегда стараемся следовать стандартным паттернам FW для методов, у которых есть аналоги из FW. Словари, методы TryParse и т.д.
При этом мы жертвуем юзабельностью ради минимума WTF moments.
В обсуждаемом случае речь идёт о самостоятельном мини-фреймворке и здесь вполне можно позволить себе отступление от стандартных рекомендаций.

3. Больше теряем, чем выигрываем.
В CJ есть public api, которое использует var result = TryDoSmth().
Примеры —
var a = Range.TryCreate(from, to);
var b = EnumHelper.TryParse<TEnum>(string name, ignoreCase: false);

Что делаем с ними? Ломаем public API, или оставляем код как есть?

Здравствуйте, Sinix, Вы писали:

Я за консистентность.
Сейчас её нет.

Предлагаю:
Методы не должны возвращать null.
Если надо вернуть null, называем TryGet(out), либо называть Get..OrDefault

Здравствуйте, _NN_, Вы писали:

_NN>Я за консистентность.
Уже ответил, см "2. Нет реальных причин для изменений." и "3. Больше теряем, чем выигрываем."

Мы выбираем не между консистентным и неконсистентным подходом, а между
1. Метод-фабрика выглядит похожим на метод для получения значения из словаря, но из метода убираются optional args.
2. Метод-фабрика заточен под реальные сценарии использования, но не похож на получение значения из словаря.

У варианта 2 минусов меньше, кмк.

_NN>Если надо вернуть null, называем TryGet(out), либо называть Get..OrDefault
Не работает с любыми нетривиальными методами. Имена вида

var a = x.ParseOrDefault();
var b = x.ReadConnectionStringOrDefault();
etc.

кмк выглядят монструозно.

Здравствуйте, Sinix, Вы писали:

S>Что делаем с ними? Ломаем public API, или оставляем код как есть?

Мы ж вроде договорились — TryXxx с out параметрами, а если null или nullable, тогда GetXxxOrDefault.

... << RSDN@Home 1.0.0 alpha 5 rev. 0 on Windows 8 6.2.9200.0>>

Здравствуйте, Sinix, Вы писали:

_NN>>Если надо вернуть null, называем TryGet(out), либо называть Get..OrDefault
S>Не работает с любыми нетривиальными методами. Имена вида
S>

S>var a = x.ParseOrDefault();
S>var b = x.ReadConnectionStringOrDefault();
S>etc.
S>

S>кмк выглядят монструозно.

TryParse(x, out var a)
TryReadConnectionString(out var b)

Итого увеличение кода на всего нв 6 символов.
Зато меньше NRE в коде.

Здравствуйте, AndrewVK, Вы писали:

S>>Что делаем с ними? Ломаем public API, или оставляем код как есть?

AVK>Мы ж вроде договорились — TryXxx с out параметрами, а если null или nullable, тогда GetXxxOrDefault.
Для большинства случаев так и сделано, но есть разумные исключения.

Примеры:

var range = Range.CreateOrDefault(1,2);
// vs 
var range = Range.TryCreate(1,2);

if (range.IsNotEmpty) { … }

var y = MappingSchema.GetConvertExpressionOrDefault(a,b);
// vs
var y = MappingSchema.TryGetConvertExpression(a,b);

    var config = metadataSource?.TryGetMetadataAttribute<ICompetitionConfigSource>()?.Config;
    // ...

Кроме этих есть ещё несколько юз-кейсов, когда out не работают, а OrDefault не видно из-за длинных имён методов. Привёл в стартовом посте.

Здравствуйте, _NN_, Вы писали:

_NN>Итого увеличение кода на всего нв 6 символов.
_NN>Зато меньше NRE в коде.

Если используется решарпер — без разницы. Если не используется, то без разницы будет после выхода c#8 и nullable refs

В общем ок, аргументацию услышал, посмотрим, что остальная команда предложит.

Здравствуйте, Sinix, Вы писали:

S>Здравствуйте, _NN_, Вы писали:

_NN>>Итого увеличение кода на всего нв 6 символов.
_NN>>Зато меньше NRE в коде.

S>Если используется решарпер — без разницы. Если не используется, то без разницы будет после выхода c#8 и nullable refs

До C# 8 дожить надо.
ReSharper не у всех.
Как раз он и навёл меня на этот код. В некоторых местах нет проверок на null и получаем потенциальный NRE.

S>В общем ок, аргументацию услышал, посмотрим, что остальная команда предложит.

Здравствуйте, Sinix, Вы писали:

Попробуем голосование

ResultType GetSomething(..., ResultType defaultValue)
{
 if(...) return defaultValue;
 else return someValue;
}

Здравствуйте, ·, Вы писали:

S>>Предложение — заменить (все?) такие методы на FW-style bool TryGet(..., out var result). Если неправильно понял или криво сформулировал — пиши, поправлю.
·>А как насчёт такого?
Проблема в том, что TryXyz() методы обычно предполагают проверку результата.
Для return null | return bool проверка делается прямо в цепочке вызовов, не отходя от кассы.
Для default value надо приседать с ternary op, т.е. он проигрывает первым двум.

Здравствуйте, Sinix, Вы писали:

S>>>Предложение — заменить (все?) такие методы на FW-style bool TryGet(..., out var result). Если неправильно понял или криво сформулировал — пиши, поправлю.
S>·>А как насчёт такого?
S>Проблема в том, что TryXyz() методы обычно предполагают проверку результата.
S>Для return null | return bool проверка делается прямо в цепочке вызовов, не отходя от кассы.
S>Для default value надо приседать с ternary op, т.е. он проигрывает первым двум.
Не понял твой аргумент... Я имею в виду, что использоваться будет так, что проверка становится вообще не нужна, т.к. дефолтное значение задаётся явно в месте вызова:

 var workers1 = consoleArgs.GetValue(_workersCountArg, "1").ToInt32Invariant()

Те же цепочки, но проще.
А с т.з. пользователя API он сможет сам выбирать использовать null или что угодно если пожелает:

 var nullableValue = consoleArgs.GetValue(_workersCountArg, null);
 var valueOrDef = consoleArgs.GetValue(_workersCountArg, default);

Здравствуйте, ·, Вы писали:

А почему не GetSomethingOrDefault по аналогии с другими подобными методами ?

GetValueOrDefault

Здравствуйте, _NN_, Вы писали:

_NN>А почему не GetSomethingOrDefault по аналогии с другими подобными методами ?
Ты имеешь в виду добавить префикс "OrDefault"? Ну не знаю, чисто вкусовщина, имхо. Как по мне — выглядит длиннее, а семантики добавляет не особо много. Даже немного запутывает, ибо в c# под "default" может пониматься ключевое слово "default".

Здравствуйте, ·, Вы писали:

·>Не понял твой аргумент... Я имею в виду, что использоваться будет так, что проверка становится вообще не нужна, т.к. дефолтное значение задаётся явно в месте вызова:

Мы вроде обсуждаем более-менее универсальное решение.

То, что ты предлагаешь, не работает для боль-менее сложных сценариев Например, если default value должно вычисляться только при отсутствии результата.
Ещё сценариев: нельзя сделать параметр с default value optional, нельзя использовать params, надо следить, чтобы пользователь не перепутал перегрузки, если в одной из них нет default value etc.
По этим причинам варианты с return bool и return null встречаются очень часто, а вот использование default arg я видел только для получения значений из option и словарей.

Здравствуйте, Sinix, Вы писали:

S>·>Не понял твой аргумент... Я имею в виду, что использоваться будет так, что проверка становится вообще не нужна, т.к. дефолтное значение задаётся явно в месте вызова:
S>Мы вроде обсуждаем более-менее универсальное решение.
S>То, что ты предлагаешь, не работает для боль-менее сложных сценариев Например, если default value должно вычисляться только при отсутствии результата.
А оно реально надо? Сложный сценарий описывать однострочно? Ну нафиг. Но если уж очень так хочется, то можно завернуть в лямбду:

var workers1 = consoleArgs.GetValue(_workersCountArg, () => "1").ToInt32Invariant()

Но вообще, по-моему если надо сложный сценарий, делаешь явно с проверкой null/default или ещё как-то по-другому декомпозируешь. Мы ж, надеюсь, за понятность кода боремся, а не за минимизацию числа буковок.

S>Ещё сценариев: нельзя сделать параметр с default value optional, нельзя использовать params, надо следить, чтобы пользователь не перепутал перегрузки, если в одной из них нет default value etc.
Если я правильно понял, что ты имеешь в виду, то это проблема любого метода с перегрузками и опционалами, а не конкретного подхода с GetValue(..., defaultValue). Просто не используй перегрузку, если результат неочевиден.

S>По этим причинам варианты с return bool и return null встречаются очень часто, а вот использование default arg я видел только для получения значений из option и словарей.
return bool + out это вообще какая-то профанация. Никогда не понимал почему это везде в c# накрутили. Что мешает использовать result в false условии?
return null это да... в общем-то работает, но проблему не решает.

Здравствуйте, Sinix, Вы писали:

S>Камрад @NN__ поднял в реквесте вопрос про возврат null из TryXxx-методов.
S>Предложение — заменить (все?) такие методы на FW-style bool TryGet(..., out var result). Если неправильно понял или криво сформулировал — пиши, поправлю.

out был оправдан только для ValueType, у которых не может быть null. Там нет возможности вернуть null, а проверить удачно или нет все-равно как-то нужно.

Если же можно вернуть null — то нафиг out? out — это такой костыль на самом деле.

Причем не все методы, которые возращают null, должны иметь префикс Try. Только те, где ожидаем некорректные данные (как то парсинг). Если же не ожидаем некорректных данных — то либо вернуть null без всяких Try либо исключение (описать поведение в документации).

Здравствуйте, Shmj, Вы писали:

S>out был оправдан только для ValueType, у которых не может быть null. Там нет возможности вернуть null, а проверить удачно или нет все-равно как-то нужно.

S>Если же можно вернуть null — то нафиг out? out — это такой костыль на самом деле.

S>Причем не все методы, которые возращают null, должны иметь префикс Try. Только те, где ожидаем некорректные данные (как то парсинг). Если же не ожидаем некорректных данных — то либо вернуть null без всяких Try либо исключение (описать поведение в документации).
А потом ловить NRE ?
Я хотел добавить аннотации к коду и выяснил, что многие методы не помечены как возвращающие null (CanBeNull), а таки null возвращают и это нигде не проверяется.

out конечно тоже не панацея, было бы лучше возвращать какой-нибудь Maybe<T>, но в C# такого нет, да и не факт, что можно сделать таким же быстрым как простое возвращение ссылочного типа.

Здравствуйте, ·, Вы писали:

S>>По этим причинам варианты с return bool и return null встречаются очень часто, а вот использование default arg я видел только для получения значений из option и словарей.
·>return bool + out это вообще какая-то профанация. Никогда не понимал почему это везде в c# накрутили. Что мешает использовать result в false условии?
·>return null это да... в общем-то работает, но проблему не решает.
Ничего не мешает использовать результат в случае возврата false.
Просто не осилили монады

Здравствуйте, _NN_, Вы писали:

S>>>По этим причинам варианты с return bool и return null встречаются очень часто, а вот использование default arg я видел только для получения значений из option и словарей.
_NN>·>return bool + out это вообще какая-то профанация. Никогда не понимал почему это везде в c# накрутили. Что мешает использовать result в false условии?
_NN>·>return null это да... в общем-то работает, но проблему не решает.
_NN>Ничего не мешает использовать результат в случае возврата false.
_NN>Просто не осилили монады Image: xz.gif

Это когда везде лямбды лямдами погоняют? Тоже не фонтан.

	От:	Sinix
	Дата:	24.10.18 17:20
	Оценка:

От:	_NN_	www.nemerleweb.com
Дата:	24.10.18 17:32
Оценка:	+1

	От:	Sinix
	Дата:	24.10.18 17:43
	Оценка:

От:	AndrewVK	http://blogs.rsdn.org/avk
Дата:	24.10.18 17:47
Оценка:

От:	_NN_	www.nemerleweb.com
Дата:	24.10.18 18:13
Оценка:	+1

	От:	·
	Дата:	29.10.18 22:48
	Оценка:	23 (1)

	От:	Shmj
	Дата:	01.11.18 17:22
	Оценка: