Re[2]: Вариант 0K - .NET

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

0K>>Главная загвоздка в данном задании -- целая гармошка возможных исключений при работе с файлами. И по идее (которую даже сами MS не выполняют)
S>Главная загвоздка в задании в том что ты списал требования со своего решения. Потому все комментарии в плоскости соответствия кода решаемой задачи бессмысленны.

Свое решение наваял вчера на сонную голову.

0K>> -- нельзя использовать Exception. Я решил так и оставить эту гармошку (5 исключений) в 2-х местах, т.к. в .Net нет вменяемых средств для устранения дублирования и повышения читаемости такого кода.
S>Видимо претензия к дотнету заключается в отсутствии классификации исключений, заточенную под твое решение этой задачи.

Не только этой задачи. Всех задач.

0K>>Весь смысл кода -- обернуть эту гармошку в одно простое исключение, ясно отражающее причину ошибки и содержащее всю необходимую информацию.
S>От того что ты обернул исключение своим, причина ошибки яснее не стала и информации не добавилось. Единственное что ты достиг оборачиванием — сгруппировал интересующие тебя типы исключения в единый. А это можно было достичь и без гармошек.

Да, в этом ошибся. Нужно было добавить инфы.

S>Еще твои гармошки можно сократить если внимательно почитать документацию к File.Exists.

Его вообще нужно убрать. Там перехват Exception, что нарушает правила.

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

N>кстати, да — конструкция throw new CounterException(exception.Message, exception); ничего не добавляет по сути. причём странновастый момент в том, что в стеке ошибок exception.Message будет повторен дважды.

Да, там нужно добавить инфы, ошибся на сонную голову. Выше уже отписал.

N>И опять же — ответственность по выдаче текста, который реально может помочь при решении проблемы лежит в классе-пользователе.

Вот это нужно исправить.

N>Т.е. класс Counter рассчитывает на то, что его будут использовать определённым способом. А если нет — то ошибка будет неочевидна. Кстати, мелкомягкий код этим не страдает (если уж брать его за эталон).

В данном случае нет смысла требовать последовательности вызовов Load Increment Save. Можно что-то пропустить --- это дело вызывающего кода. А вот сообщения в класс Counter добавить нужно, иначе будет дублирование кода.

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

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

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

G>>>

G>>>        public Counter(string alias)
G>>>        {
G>>>            if (null == alias)
G>>>                throw new ArgumentNullException("alias");

G>>>            if (string.IsNullOrEmpty(alias))
G>>>                throw new ArgumentOutOfRangeException("alias");
             
U>>             ....

G>>>        }
G>>>

U>>Так какой же Exception должен выбрасываться при alias == null ?

G>Это мне вопрос? Я скопипастил.

я знаю.

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

G>>>>

G>>>>        public Counter(string alias)
G>>>>        {
G>>>>            if (null == alias)
G>>>>                throw new ArgumentNullException("alias");

G>>>>            if (string.IsNullOrEmpty(alias))
G>>>>                throw new ArgumentOutOfRangeException("alias");
             
U>>>             ....

G>>>>        }
G>>>>

U>>>Так какой же Exception должен выбрасываться при alias == null ?

G>>Это мне вопрос? Я скопипастил.

U>я знаю.

Всё верно, должен выбрасываться ArgumentNullException. Это обычный usage exception.

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

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

G>>>>>

G>>>>>            if (string.IsNullOrEmpty(alias))
G>>>>>                throw new ArgumentOutOfRangeException("alias");
G>>>>>

Q>Всё верно, должен выбрасываться ArgumentNullException. Это обычный usage exception.
Зачем повторная проверка на null ?

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

0K>>Собственно вариант и несколько комментариев: [skipped]
G>Да уж, на одну строку логики 3 строки непонятно чего.

"Непонятно чего" -- вы называете обработку исключения?

G>Кроме того нету обработки переполнения, и немалый security баг.

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

Вы, кстати, так и не смогли этого сделать.

G>Вообще код выглядит как большая ошибка проектирования.

Вы вообще не смогли ничего сделать. Так что помолчали бы. Чтобы так сказать -- нужно самому сделать правильно и показать как правильно.

G>Ты наверное не понимаешь что большинство исключений для обычной программы являются критическими,

Для разных программ соотношение критические/не критические может быть разным. И что?

G>а пользователи не вводят имена файлов.

А кто вводит?

0K>>Весь смысл кода -- обернуть эту гармошку в одно простое исключение, ясно отражающее причину ошибки и содержащее всю необходимую информацию.
G>И для этого ты два раза скопипастил код обработки? Браво!

Не внимательно смотрели. Разница есть. Но там еще нужно добавить сообщение, разное для каждого из 2-х случаев.

G>Упрощу немного твой вариант:

Вы не упростили:

1. при загрузке неверно обработали FileNotFoundException
2. перенос обработки в функцию принимающую делегаты в качестве параметров -- только усложнил восприятие. А что если код будет использовать в Win-приложении и при возникновении ошибки нужно будет некоторые элементы сделать видимыми/другие неактивными? Тогда вы введете 2 дополнительные мелкие функции, а этого лучше не делать.

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

А основную мою ошибку вы так и не исправили.

G>В итоге CounterException не нужен, потом что его роль в исходной программе — только передача сообщения. Это вполне можно делать более простыми способами. О чем я тебе и говорил все время.

Но ваш вызывающий код должен знать логику работы -- какие исключения могут возникнуть. И если вы измените тип хранилища на базу данных -- нужно будет переделывать вызывающий код. Вот так то. Об этом у вас не хватило подумать.

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

X>1. Зачем метод Counter.TryParse, если вы на том же уровне (из Program.Main) делаете обернутый counter.Load и counter.Save? Что мешало сделать такой же обернутый new Counter(alias)? Как-то непоследовательно...

Т.к. Counter.TryParse -- не возвращает дополнительной информации. В случае с сохранением/загрузкой -- может быть слишком много вариантов проблемы и все их нужно знать.

X>2. Не описана ответственность класса Counter. Можно-ли дергать за .Increment() до .Load() ? Можно-ли дергать за .Increment() после .Save()? Тут явное нарушение SRP: класс отвечает как за инкремнтирование счетчика, так и за работу с файловым хранилищем.

Вообще-то идея разделить ответственность у меня возникала, но для такой простой задачи -- лишнее. В данном случае можно дергать методы без всяких ограничений -- нарушения не будет, это дело вызывающего кода (если ему так нужно -- пусть дергает сначала Save, потом Increment, потом опять Save).

X>3. Counter.Increment() гаранитрует успешеное выполнение ?

Да.

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

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

G>>

G>>        public Counter(string alias)
G>>        {
G>>            if (null == alias)
G>>                throw new ArgumentNullException("alias");

G>>            if (string.IsNullOrEmpty(alias))
G>>                throw new ArgumentOutOfRangeException("alias");
             
U>             ....

G>>        }
G>>

U>Так какой же Exception должен выбрасываться при alias == null ?

ArgumentNullException. А что?

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

G>Это мне вопрос? Я скопипастил.

Вы же переделали. НЕ пытайтесь избежать ответственности за свой код.

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

U>Так какой же Exception должен выбрасываться при alias == null ?

Меня куда больше удивляет ArgumentOutOfRangeException

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

U>Зачем повторная проверка на null ?

Это не повторная проверка на null. Это независимая проверка. Проверка того, что входные данные уже были подготовлены, контракты соблюдены.
Эти проверки в общем случае осуществляются разными людьми — автором библиотеки и пользователем библиотеки. По указанной выше ссылке — подробнее. (Хотя конкретно в приведённом коде я двойной проверки не вижу.)

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

Q>(Хотя конкретно в приведённом коде я двойной проверки не вижу.)

Неужто глаз уже так замылился? =)
G>> if (null == alias)
G>> if (string.IsNullOrEmpty(alias))

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

M>Неужто глаз уже так замылился? =)
G>>> if (null == alias)
G>>> if (string.IsNullOrEmpty(alias))

А, та это ничего страшного, просто вызвал товарищ стандартную функцию. Эта лишняя проверка лучше, имхо, чем сравнение Lenth с нулём, на которую FxCop (CodeAnalysis) к тому же ругается. Можно, конечно, как-нибудь !alias.Any(), но овчинка не стоит выделки.

Алсо, там могло быть ещё и третье сравнение с null'ом, типа String.IsNullOrWhitespace (фишка FCL 4.0).

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

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

M>>Неужто глаз уже так замылился? =)
G>>>> if (null == alias)
G>>>> if (string.IsNullOrEmpty(alias))

Q>А, та это ничего страшного,

Ну а теперь предположим, что в процессе разработки эти проверки перепутают местами

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

M>Ну а теперь предположим, что в процессе разработки эти проверки перепутают местами ;)

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

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

Q>Ну, эдак про любые две строчки можно сказать.

Нет, я все-таки на этих настаиваю =)

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

0K>Здравствуйте, gandjustas, Вы писали:

0K>>>Собственно вариант и несколько комментариев: [skipped]
G>>Да уж, на одну строку логики 3 строки непонятно чего.

0K>"Непонятно чего" -- вы называете обработку исключения?

G>>Кроме того нету обработки переполнения, и немалый security баг.

0K>Это не критично -- оговаривалось. В данном задание такое поведение является нормальным, т.к. главное что требуется -- корректно обработать исключения и выдать корректные сообщения.
Кто ты понимаешь по корректной обработкой исключения и под выдачей корректного сообщения?
Ты же кстати не сделал локализацию сообщений

0K>Вы, кстати, так и не смогли этого сделать.
Выдаются в точности те сообщения что у тебя

G>>Вообще код выглядит как большая ошибка проектирования.
0K>Вы вообще не смогли ничего сделать. Так что помолчали бы. Чтобы так сказать -- нужно самому сделать правильно и показать как правильно.
Повторенное дважды правдой не становится, я привел два варианта кода с разными usecase.

G>>а пользователи не вводят имена файлов.
0K>

А кто вводит?
Администратор в конфиге. Пользователь может выбрать существующий файл в диалоге.

0K>>>Весь смысл кода -- обернуть эту гармошку в одно простое исключение, ясно отражающее причину ошибки и содержащее всю необходимую информацию.
G>>И для этого ты два раза скопипастил код обработки? Браво!

0K>Не внимательно смотрели. Разница есть. Но там еще нужно добавить сообщение, разное для каждого из 2-х случаев.
Нету там разницы, везде

throw new CounterException(exception.Message, exception);

G>>Упрощу немного твой вариант:

0K>Вы не упростили:

0K>1. при загрузке неверно обработали FileNotFoundException
Да ну? И чем же неверно?

0K>2. перенос обработки в функцию принимающую делегаты в качестве параметров -- только усложнил восприятие.
А для меня упростил.

0K>А что если код будет использовать в Win-приложении и при возникновении ошибки нужно будет некоторые элементы сделать видимыми/другие неактивными?
Ну сделаю это в хендлерах.

0K>Тогда вы введете 2 дополнительные мелкие функции, а этого лучше не делать.
Счегобы? Это нарушает какие-то гайдлайны? Ты в курсе сколько гайдлайнов нарушает твой код?

0K>Ну и зря выкинули TryParse -- с ним меньше блоков и гораздо понятнее код.
Вообще надо проверку имени вынести отдельно и с помощью regexp, так сразу и security баг закроется.

0K>А основную мою ошибку вы так и не исправили.
Еще я ошибки в твоем коде искать буду... мне лениво.
Я просто показал что эквивалент твоему коду можно написать без эксепшена для передачи сообщения, при этом получилось в полтора раза короче.

А в реальной системе, при применении Exception Handling Application Block, можно всю такую обработку вынести вне логики.

G>>В итоге CounterException не нужен, потом что его роль в исходной программе — только передача сообщения. Это вполне можно делать более простыми способами. О чем я тебе и говорил все время.

0K>Но ваш вызывающий код должен знать логику работы -- какие исключения могут возникнуть.
Кому должен?

0K>И если вы измените тип хранилища на базу данных -- нужно будет переделывать вызывающий код.
Если я буду переделывать код, то я буду переделывать код вместе с обработкой ошибок.
Причем я это буду делать в одном месте, а тебе придется переделывать в двух.

0K>Вот так то. Об этом у вас не хватило подумать.
И правильно, надо решать задачу, которую надо решать, а не те которые могут возникнуть через 100 лет.

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

0K>Здравствуйте, gandjustas, Вы писали:

G>>Это мне вопрос? Я скопипастил.

0K>Вы же переделали. НЕ пытайтесь избежать ответственности за свой код.

Мой код?

Мой код тут и тут. По коду претензий не было, и тот и другой решает свои задачи.

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

0K>Здравствуйте, Аноним, Вы писали:

А>>Большинство справедливо возразило: исключения _только_ для программистов.

0K>Большинство, как правило, ошибается. Дураков большинство.

Т.к. конкурс многими не был воспринят адекватно

так бы и писали: Т.к. конкурс _дураками_ не был воспринят адекватно... Или вы не это хотели сказать?

А>>Все таки exception это такая же часть контракта, как и типы возвращаемых значений. Ты же не собираешься публиковать "парадигму типов возврата и обработки возвращаемых значений"?

0K>Exception -- не только контракт. Message контрактных исключений НИКОГДА не нужно выводить пользователю -- они только для программиста. Есть бизнес-исключения -- вот они содержат полезый для пользователя Message.

У каждого метода есть контракт: а) принимать определенные параметры б) выполнять набор действий в) возвращать определенный результат г) кидать одно из исключений при невозможности выполнить действие/вернуть результат

про особый вид "контрактных исключений" вы упоминаете впервые и ого! они тоже обладают уникальными свойствами: их message никогда! нельзя выводить пользователю. Следуя вашей категоризации у нас есть три типа exceptions:
контрактные — никогда нельзя выводить пользователю.
user-friendly — всегда выводить пользователю.
остальные.

0K>UserFriendly -- это CounterException. Его Message полезен пользователю. Хотя там можно чуть красивее сделать с Message, возможно покажу.

А>>где обязательно-принудительный вывод текста UserFriendly Exception?

0K>А разве не видно? Везде при возникновении ошибки выводится Message.
вариант 1: оно user-friendly потому что его message выводится пользователю? а если я выведу пользователю message от PathNotFoundException оно станет User-Friendly?
вариант 2: оно user-friendly поэтому его message всегда выводятся пользователю? (кстати можно или нужно?) и где тогда проверка, что пользователь получает сообщения только от user-friendly exception?
Вот потребуется продвинутый каунтер, который будет считатывать на сколько увеличивать счетчик из внешнего источника(например через веб-сервис) и при неудаче метод Counter.Increment() станет кидать CounterException — ваш код проигнорирует CounterException и вывалится с UH.

Еще раз: как можно догадаться, что CounterException user-friendly?

0K>CounterException только для вывода пользователю используется.

я бы переписал код в теле примерно так:

                try
                {
                    Console.WriteLine("Введите имя счетчика:");
                    string alias = Console.ReadLine();

                    var counter = new Counter(alias); // может бросить ArgumentException

                    var value = counter.Increment(); // может бросить CounterReadException, CounterIncrementException, CounterWriteException

                    Console.WriteLine("Текущее значение счетчика: {0}.", value);

                }
                catch (ArgumentException)
                {
                    Console.WriteLine("Неверно задано имя счетчика");
                }
                catch (CounterReadException)
                {
                    Console.WriteLine("Ошибка при получении значения счетчика.");
                }
                catch (CounterIncrementException)
                {
                    Console.WriteLine("Ошибка при увеличении значения счетчика.");
                }
                catch (CounterWriteException)
                {
                    Console.WriteLine("Ошибка при сохранении нового значения счетчика.");
                }

/// Counter
    public class Counter
    {
        private string _path;

        protected Counter() { }

        /// <exception cref="ArgumentNullException">alias is null.</exception>
        /// <exception cref="ArgumentOutOfRangeException">alias is empty</exception>
        /// <exception cref="ArgumentException">Invalid character</exception>
        public Counter(string alias)
        {
            if (null == alias)
                throw new ArgumentNullException("alias");

            if (string.IsNullOrEmpty(alias))
                throw new ArgumentOutOfRangeException("alias");

            if (alias.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0)
                throw new ArgumentException("Invalid character.", "alias");

            _path = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, alias);
        }

        /// <exception cref="CounterReadException"></exception>
        /// <exception cref="CounterWriteException"></exception>
        public int Increment()
        {
            var readValue = Load();
            var incrementedValue = InternalIncrement(readValue)
            Save(incrementedValue);
        }

        /// <exception cref="CounterReadException"></exception>
        protected int Load()
        {
            // throws CounterReadException on failure
        }

        /// <exception cref="CounterWriteException"></exception>
        protected void Save(int value)
        {
            // throws CounterWriteException on failure
        }

        /// <exception cref="CounterIncrementException"></exception>
        protected int InternalIncrement(int readValue)
        {
            // throws CounterIncrementException on failure
        }
    }

Очевидно, что за вывод пользователю отвечает Console.WriteLine, за передачу информации о неудаче операции — разные типы исключений. Normal-flow при это прозрачен и не загроможен кучей try/catch блоков.
И да: интеративный режим у консольного приложения подразумевает, что пользователю имена файлов, и прочие "вкусности" будут как "- Ой, Вань, гляди-кось, попугайчики.
Нет, я ей-Богу закричу!"

	От:	0K
	Дата:	01.09.10 10:08
	Оценка:	-1

	От:	0K
	Дата:	01.09.10 10:11
	Оценка:

	От:	0K
	Дата:	01.09.10 10:17
	Оценка:

	От:	Uzzy
	Дата:	01.09.10 10:20
	Оценка:

От:	Qbit86	https://twitter.com/qbit86
Дата:	01.09.10 10:26
Оценка:

От:	gandjustas	http://blog.gandjustas.ru/
Дата:	01.09.10 11:51
Оценка: