Как стать автором
Обновить

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

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

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

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

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

>Более того — параметров в общем случае недостаточно, потому что нет настроек прокси и шифрования, а многие почтовые сервера работают с ними.
О таком кейсе я не подумал, так как в моем случае все обошлось простейшим SMTP, но благо весь код открытый, соответственно никто не мешает доделать под соответственные нужды.
Метод, у которого больше четырех параметров выглядит грязно. Только и всего. И читать его сложнее.
Соглашусь, но ничего лучше не придумал.
> Поясните пожалуйста подробнее что вы предлагаете, так как в любом случае необходимо инициализировать все параметры, в вариант конструктора ничем по сути не отличается. Спасибо.
Ваш метод
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"
              });
Все свойства должны быть инициализированы и случае использования вашего варианта придется делать дополнительную проверку на неинициализированные свойства, что в условиях подключаемой библиотеки срабатывающей только в случае исключительной ситуации рискованно.
Как я отметил выше, вариант с конструктором по сути ничем не отличается.
В таком случае, как минимум, необходима проверка на правильность переданных данных, что логичней сделать internal методом IsValid в классе Settings, где и будет находиться вся логика проверки на инициализированность и правильность:

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

Соглашусь, можно было сделать так.
Вариант неплох, но проблема, что в данном случае библиотека, имеющая ошибку на этапе компиляции, будет выбрасывать исключение в рантайме. Да и вообще, библиотека обработки ошибок, бросающая исключения, это немного странно. Вариант ниже обладает теми же достоинствами, но исключает эти недостатки — создать объект можно только со всеми параметрами, а затем менять их можно только на валидные.
Получилось достаточно интересное Code Review!
Также, я бы хотел предложить всем желающим поучаствовать в развитие проекта написать мне в личку или на почту.
Предоставлю права на commit.
Спасибо.
Можно сделать конструктор с обязательными параметрами, а свойства сделать либо read-only, либо меняющие свое значение только при валидных данных (то есть не авто-свойства, а обычные свойства с какой-то логикой). Тогда settings будет всегда в валидном состоянии (такой вот транзакционный подход).
Про то, что параметры следует выделить в отдельный класс согласен.
Не обязательно все параметры выставлять в конструкторе, можно сдлетаь объект с Fluent интерфейсом, например. И это не единственный способ.
наверное имеет смысл «сообщение с просьбой отправить лог файл разработчику на почту» показывать после «сохраняем ошибку в лог файл»
Исправил, спасибо!
Вам совет, и не только вам. Лучше не используйте ILMerge, если не хотите проблем.
Использую уже во втором проекте, каких-либо проблем не испытываю.
Не могли вы поподробнее рассказать с какими проблемами можно столкнуться?
Спасибо.
Все очень просто. Как работает ILMerge: он читает полностью весь IL код в память, а потом пишет в отдельную сборку. Код, который использует IlMerge, написан в MS Research и не имеет ничего общего с компилятором. То, что на данном этапе все работает для вас — это хорошо, но я бы не стал на ILMerge сильно надеяться и использовать его везде, где только можно.
Основные проблемы начинаются там, где присутствует Type Forwarding. Один из примеров — Portable Class Libraries. А так же Библиотеки из разных версий .NET Framework 3.5 -> 4.0 -> 4.5 и т.п.
Большая просьба тем кто проставил минусы статье и мне прокомментировать что именно не понравилось, так как судя по добавлениям в избранное статья все-таки кому-то полезна. Спасибо.
Также хочется отметить, что я стараюсь расшарить свои наработки с сообществом, теми решениями которые могут полезны, с той целью чтобы мои труды кому-то помогли как не раз чьи-то статьи выручали меня.
Я не уверена, но кажется статья слабенькая, по этому и минусуют.
Много сделано как «так делать нельзя», если вы меня понимаете.

А вообще, посмотрите в сторону WER (Windows Error Reporting).
Спасибо за комментария, правда я не понял почему статья слабенькая.
Это узкоспециализированная статья о моем бесплатном open source проекте, рассчитанная на .NET разработчиков.
В статье присутствует описание проекта со скриншотами, инструкция по использованию и объяснение как это работает.
Хотя конечно соглашусь, сложно сравнивать узкоспециализированную статью, от который я в большинстве своем получаю только минусы, с популярными на хабре обсуждениями политики, космонавтики, новых гаджетов и прочими популярными темами.
> Спасибо за комментария, правда я не понял почему статья слабенькая.
Не уровень Голливуда, видимо.
Стараюсь как могу.
Знакомая задача. У нас похожая форма, только приложения одновременно и WinForms и WPF, причём какие-то проекты это WinForms хостящий WPF, а какие-то WPF хостящий WinForms. Пришлось навешиваться на обе темы одновременно, а окошко с уведомлением об ошибке делать на WPF.

Да, при обработке исключений мы ещё проверяем, что к проекту уже приаттачен отладчик и в этом случае окошко не открываем — от отладчика разработчик узнает гораздо больше, чем из пользовательского окошка.
Забавно, но я делаю точно также как и вы, это можно увидеть в моей прошлой статье.
if (!Debugger.IsAttached)    { AppDomain.CurrentDomain.UnhandledException += (sender, e) => HandleError((Exception)e.ExceptionObject);}

Но в данном случае, по какой-то причине, в случае ошибки в приложении окно ошибки не показывается, пока еще не разбирался почему.
Нене, навешивать обработчик на необработанные исключения нужно в любом случае, а вот при обработке исключения уже стоит проверять на наличие отладчика. Отладчик может подключиться и отключиться в любой момент.
делал аналогичную формочку, но мы до кучи еще скриншот делали, иногда стек-трейса недостаточно.
Отличная идея, надо будет поиграться с этим.
От проекта зависит, но на скриншот может попасть персональная информация чему пользователь скорее всего будет не рад.
ну если в проекте все так серьезно, можно сделать галочку или кнопочку которая скриншот удалять будет.
еще полезно бывает логи приложения (не только стек) и app.config прилепить.
Зарегистрируйтесь на Хабре , чтобы оставить комментарий

Публикации

Истории