Diaver7 мая 2014 в 17:50

Окно сообщения об ошибке для WinForms и WPF приложений

4 мин

20K

Программирование * .NET *

Туториал

+16

Комментарии 34

bobermaniac 7 мая 2014 в 18:44

Семь параметров в методе — у вас рак параметров.

Более того — параметров в общем случае недостаточно, потому что нет настроек прокси и шифрования, а многие почтовые сервера работают с ними.

Я бы рекомендовал (раз уж у нас Winforms/WPF) использовать Outlook Automation, и если он отсутствует — только тогда идти через отправку напрямую по SMTP.

А параметры упаковать в класс — его будет проще расширить, когда параметров станет 20.

mayorovp 8 мая 2014 в 11:01

А если я использую другой почтовый клиент при установленном Outlook?

bobermaniac 8 мая 2014 в 21:44

Ну, я могу вам посочувствовать.

Diaver 7 мая 2014 в 18:59

>А параметры упаковать в класс — его будет проще расширить, когда параметров станет 20.
Поясните пожалуйста подробнее что вы предлагаете, так как в любом случае необходимо инициализировать все параметры, в вариант конструктора ничем по сути не отличается. Спасибо.

>Более того — параметров в общем случае недостаточно, потому что нет настроек прокси и шифрования, а многие почтовые сервера работают с ними.
О таком кейсе я не подумал, так как в моем случае все обошлось простейшим SMTP, но благо весь код открытый, соответственно никто не мешает доделать под соответственные нужды.

bobermaniac 7 мая 2014 в 19:27

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

Diaver 7 мая 2014 в 19:28

Соглашусь, но ничего лучше не придумал.

itsplus 8 мая 2014 в 08:47

del, уже посоветовали

vyacheslav_ka 7 мая 2014 в 19:32

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

public static class UIException
    {
        public static void Start(string serverSmtp, int portSmtp, string passwdSmtp, string userSmtp, string programmerEmail, string fromEmail, string exceptionSubject)
        {
            Settings.ServerSMTP = serverSmtp;
            Settings.PortSMTP = portSmtp;
            Settings.PasswdSMTP = passwdSmtp;
            Settings.UserSMTP = userSmtp;
            Settings.ProgrammerEmail = programmerEmail;
            Settings.FromEmail = fromEmail;
            Settings.ExceptionSubject = exceptionSubject;

Можно сделать так:

public static class UIException
    {
        public static void Start(Settings settings)
        {

Тогда вызов метода будет выглядеть как:

UIExceptionHandlerWPF
       .UIException
       .Start(new Settings {
                ServerSMTP: "SmtpServer",
                PortSMTP:  26,
                PasswdSMTP: "Password",
                UserSMTP: "User",
                ProgrammerEmai: "developer@gmail.com",
                FromEmail: "user@gmail.com",
                ExceptionSubject:  "Exception"
              });

Diaver 7 мая 2014 в 19:38

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

vyacheslav_ka 7 мая 2014 в 20:00

В таком случае, как минимум, необходима проверка на правильность переданных данных, что логичней сделать internal методом IsValid в классе Settings, где и будет находиться вся логика проверки на инициализированность и правильность:

public static class UIException
    {
        public static void Start(Settings settings)
        {  
            if(!settings.IsValid())
               throw new Exception('Settings is not valid!');

Diaver 7 мая 2014 в 20:02

Соглашусь, можно было сделать так.

PsyHaSTe 8 мая 2014 в 22:51

Вариант неплох, но проблема, что в данном случае библиотека, имеющая ошибку на этапе компиляции, будет выбрасывать исключение в рантайме. Да и вообще, библиотека обработки ошибок, бросающая исключения, это немного странно. Вариант ниже обладает теми же достоинствами, но исключает эти недостатки — создать объект можно только со всеми параметрами, а затем менять их можно только на валидные.

Diaver 9 мая 2014 в 19:03

Получилось достаточно интересное Code Review!
Также, я бы хотел предложить всем желающим поучаствовать в развитие проекта написать мне в личку или на почту.
Предоставлю права на commit.
Спасибо.

PsyHaSTe 8 мая 2014 в 22:48

Можно сделать конструктор с обязательными параметрами, а свойства сделать либо read-only, либо меняющие свое значение только при валидных данных (то есть не авто-свойства, а обычные свойства с какой-то логикой). Тогда settings будет всегда в валидном состоянии (такой вот транзакционный подход).

jmistx 20 мая 2014 в 15:30

Про то, что параметры следует выделить в отдельный класс согласен.
Не обязательно все параметры выставлять в конструкторе, можно сдлетаь объект с Fluent интерфейсом, например. И это не единственный способ.

GubkaBob 7 мая 2014 в 20:20

наверное имеет смысл «сообщение с просьбой отправить лог файл разработчику на почту» показывать после «сохраняем ошибку в лог файл»

Diaver 7 мая 2014 в 20:22

Исправил, спасибо!

outcoldman 7 мая 2014 в 20:24

Вам совет, и не только вам. Лучше не используйте ILMerge, если не хотите проблем.

Diaver 7 мая 2014 в 20:26

Использую уже во втором проекте, каких-либо проблем не испытываю.
Не могли вы поподробнее рассказать с какими проблемами можно столкнуться?
Спасибо.

outcoldman 7 мая 2014 в 20:55

Все очень просто. Как работает ILMerge: он читает полностью весь IL код в память, а потом пишет в отдельную сборку. Код, который использует IlMerge, написан в MS Research и не имеет ничего общего с компилятором. То, что на данном этапе все работает для вас — это хорошо, но я бы не стал на ILMerge сильно надеяться и использовать его везде, где только можно.
Основные проблемы начинаются там, где присутствует Type Forwarding. Один из примеров — Portable Class Libraries. А так же Библиотеки из разных версий .NET Framework 3.5 -> 4.0 -> 4.5 и т.п.

Diaver 7 мая 2014 в 20:55

Спасибо за разъяснение!

Diaver 7 мая 2014 в 22:14

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

Angelina_Joulie 8 мая 2014 в 05:06

Я не уверена, но кажется статья слабенькая, по этому и минусуют.
Много сделано как «так делать нельзя», если вы меня понимаете.

А вообще, посмотрите в сторону WER (Windows Error Reporting).

Diaver 8 мая 2014 в 05:41

Спасибо за комментария, правда я не понял почему статья слабенькая.
Это узкоспециализированная статья о моем бесплатном open source проекте, рассчитанная на .NET разработчиков.
В статье присутствует описание проекта со скриншотами, инструкция по использованию и объяснение как это работает.
Хотя конечно соглашусь, сложно сравнивать узкоспециализированную статью, от который я в большинстве своем получаю только минусы, с популярными на хабре обсуждениями политики, космонавтики, новых гаджетов и прочими популярными темами.

andreycha 8 мая 2014 в 07:59

> Спасибо за комментария, правда я не понял почему статья слабенькая.
Не уровень Голливуда, видимо.

Diaver 8 мая 2014 в 09:58

Стараюсь как могу.

andreycha 8 мая 2014 в 18:51

Не обижайтесь, это была отсылка к нику выше :).

OlegKozlov 8 мая 2014 в 05:21

Знакомая задача. У нас похожая форма, только приложения одновременно и WinForms и WPF, причём какие-то проекты это WinForms хостящий WPF, а какие-то WPF хостящий WinForms. Пришлось навешиваться на обе темы одновременно, а окошко с уведомлением об ошибке делать на WPF.

Да, при обработке исключений мы ещё проверяем, что к проекту уже приаттачен отладчик и в этом случае окошко не открываем — от отладчика разработчик узнает гораздо больше, чем из пользовательского окошка.

Diaver 8 мая 2014 в 05:31

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

if (!Debugger.IsAttached)    { AppDomain.CurrentDomain.UnhandledException += (sender, e) => HandleError((Exception)e.ExceptionObject);}

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

OlegKozlov 8 мая 2014 в 05:44

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

itsplus 8 мая 2014 в 08:50

делал аналогичную формочку, но мы до кучи еще скриншот делали, иногда стек-трейса недостаточно.

Diaver 8 мая 2014 в 08:52

Отличная идея, надо будет поиграться с этим.

KindDragon 8 мая 2014 в 09:17

От проекта зависит, но на скриншот может попасть персональная информация чему пользователь скорее всего будет не рад.

itsplus 8 мая 2014 в 09:25

ну если в проекте все так серьезно, можно сделать галочку или кнопочку которая скриншот удалять будет.
еще полезно бывает логи приложения (не только стек) и app.config прилепить.

Зарегистрируйтесь на Хабре, чтобы оставить комментарий