Читабельность
От: Jolly Roger  
Дата: 29.08.10 07:33
Оценка:
Допустим, есть функция, возвращающая bool,
static extern bool Some();

и два варианта записи проверки её результата:

[1]
static void Check(bool R) { if (!R) throw new Win32Exception(); }

[2]
static class BoolExt
{
    public static void Check(this bool R)  { if (!R) throw new Win32Exception(); }
}


Какой из следующих вариантов вызова воспринимается, на ваш взгляд, лучше:

Check(Some());

Some().Check();


По мне, например, первый выглядит привычнее, но привычка — штука коварная
"Нормальные герои всегда идут в обход!"
Re: Читабельность
От: Аноним  
Дата: 29.08.10 08:21
Оценка: 12 (1) +1
Здравствуйте, Jolly Roger, Вы писали:

Во первых, по поводу читабельности — функции, бросающие исключения в случае ложного условия обычно называются "утверждения" — assertions.
Поэтому, имхо, читабельные варианты должны придерживаться этого правила:
Assert.IsTrue(Some())
Assert.That(Some())


Именуйте методы нормально и люди к вам потянутся .
Вариант
Some().AssertTrue()

по сравнению с выше приведенными смотрится довольно натянуто, обычно в таких выражениях главное — сам факт проверки, а при таком подходе он оказывается в конце. Кроме того, если условие — вызов функции с побочными эффектами, то операция обычно все же разбивается на две части: получение значения и асерт, т.к. последние в некоторых языках вырезаются в релизе.
Re: Читабельность
От: Sinix  
Дата: 29.08.10 09:04
Оценка: 12 (1)
Здравствуйте, Jolly Roger, Вы писали:

JR>Какой из следующих вариантов вызова воспринимается, на ваш взгляд, лучше:


Я за 1й вариант. Уже ответили выше, дополню.

У нас подобные методы сгруппированы в отдельные static-классы, оканчивающиеся на Code. Имена методов соответствуют проверяемым утверждениям (Code.NotNull(); PathCode.ValidPath() etc). Используются полностью аналогично CodeContracts (в планах, как контракты допилят, перевод на них методов-ассертов). В результате в коде практически нет тонн копипасты вида
if(bla.Bla())
{
  throw new OlalaException(Resources.DummyMessage);
}
// Repeat until getting miserable.


Контракты стоит явно отделять от логики работы программ по нескольким причинам.

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

Во-вторых, уже существует стандартный стиль для подобного API (Debug.Assert, Trace.WriteLine etc).

В-третьих, со временем у вас накопится множество методов для проверки. Если оформить ассерты методами-расширениями, вы получите множество лишних элементов в автокомплите.

Наконец, стиль с явной секцией — описанием контракта провоцирует проверять предусловия в начале метода, тем самым не давая свалиться посередине работы.
Re[2]: Читабельность
От: Jolly Roger  
Дата: 29.08.10 09:46
Оценка:
Здравствуйте, Аноним, Вы писали:
Здравствуйте, Sinix, Вы писали:

Спасибо, я, в принципе, тоже так думаю, хотя и не со всеми деталями согласен От Assert за версту несёт отладкой, здесь-же вполне боевая работа, функцию возвращает bool и с этим ничего не поделаешь, мне же нужно исключение. То есть семантика — "проверить результат вызова", потому CheckXYZ мне представляется вполне разумным именем.

Почему собственно возникли сомнения — хотя первый вариант для меня привычен, но он несколько затеняет логику — всё-таки первично и значимо здесь — вызов функции, проверка-же с генерацией исключения — вторична, чисто инфраструктурный компонент. Разумеется, наружу ничего подобного торчать не будет, но и внутренности иногда нужно читать Вероятно, лучше всего было-бы добавить на каждую фунцию заглушку, генерирующую исключение и забыть о внутренностях, но, к сожалению, не всегда это годится.
"Нормальные герои всегда идут в обход!"
Re[3]: Читабельность
От: Sinix  
Дата: 29.08.10 10:14
Оценка:
Здравствуйте, Jolly Roger, Вы писали:

JR>От Assert за версту несёт отладкой.


Необязательно, с появлением CodeContracts это обычная практика. Учитывая сколько кода этими контрактами напичкано — даже почти реальность

Я ж писал выше, контракты, по сути, высокоуровневая обёртка над if() throw new ..., с мегабонусом в виде статической верификации.

JR>Почему собственно возникли сомнения — хотя первый вариант для меня привычен, но он несколько затеняет логику — всё-таки первично и значимо здесь — вызов функции, проверка-же с генерацией исключения — вторична, чисто инфраструктурный компонент.


Я вам говорил про отделение логики от контракта? "Правильно" — это примерно так:
bool someMethodSucceed = SomeMethod();

Code.AssertState(someMethodSucceed, someMessage);

// ...
Re[4]: Читабельность
От: Jolly Roger  
Дата: 29.08.10 10:57
Оценка: +1
Здравствуйте, Sinix, Вы писали:

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


JR>>От Assert за версту несёт отладкой.


S>Необязательно, с появлением CodeContracts это обычная практика. Учитывая сколько кода этими контрактами напичкано — даже почти реальность


Ну, тем CodeContract ещё самим до понятия "обычная практика" далековато В MS что ни придумают, то супер-пупер панацея от всех бед, одних только побед над DLL Hell сколько было, и где оно всё теперь? Тем более, что там assert выглядит органично именно по причине наличия статической верификации. Но это конечно субъективно.

S>Я ж писал выше, контракты, по сути, высокоуровневая обёртка над if() throw new ..., с мегабонусом в виде статической верификации.


JR>>Почему собственно возникли сомнения — хотя первый вариант для меня привычен, но он несколько затеняет логику — всё-таки первично и значимо здесь — вызов функции, проверка-же с генерацией исключения — вторична, чисто инфраструктурный компонент.


S>Я вам говорил про отделение логики от контракта? "Правильно" — это примерно так:

S>
S>bool someMethodSucceed = SomeMethod();

S>Code.AssertState(someMethodSucceed, someMessage);

S>// ...
S>


То есть надо писать две строки вместо одной? Ну для меня это точно хуже читается.
Говорили, да только оно тут не в теме, ИМХО Контрактом будет
void Some() { Check(_some()); };

или, если отрицательный результат имеет смысл при построении логики

bool SomeFunc();
void Some(){ Check(SomeFunc()); }


То есть наружу будет торчать вполне самодостаточные методы, не требующие при вызове использования ни Cheсk'ов, ни Assert'ов.
Речь-же шла о построении внутренней инфраструктуры, в которой понятие контракта, пожалуй, не совсем уместно.
"Нормальные герои всегда идут в обход!"
Re[5]: Читабельность
От: Sinix  
Дата: 29.08.10 11:04
Оценка: +1
Здравствуйте, Jolly Roger, Вы писали:

Чтоб не плодить энтропию: в основном согласен, по мелочам препираться не стоит.

Удачи
Re: Читабельность
От: Pro100Oleh Украина  
Дата: 29.08.10 13:47
Оценка: 12 (2)
Я приделживаюсь такого стиля:

    class Program
    {

        static void Main(string[] args)
        {
            try
            {
                EntitySource source = new EntitySource();
                EntityWorker worker = new EntityWorker();

                //all work in one line
                worker.WorkWithEntity(source.GetEntity().AssertNotNull());
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex);
            }
        }

    }

    public class Entity
    {
    }

    public class EntitySource
    {
        public Entity GetEntity()
        {
            return new Entity();
        }
    }

    public class EntityWorker
    {
        public void WorkWithEntity(Entity entity)
        {
            //some work with entity
        }
    }

    public static class EntityExt
    {
        public static Entity AssertNotNull(this Entity entity)
        {
            if (entity == null) throw new ArgumentNullException();
            return entity;
        }
    }
Pro
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.