Комментарии 16
Я даже не знаю, что хуже — ваш LoginzaHelper сам по себе, или тот код, которым вы его используете
А что вам не понравилось?
я не очень то шибко разбираюсь в MVC, но:
а) catch { return View(«Error»); } — нужно менять, например, на throw new HttpException(500);, да и вообще не смысла перехватывать, это сделает сама инфраструктура MVC
б) if (token == null) throw new ArgumentNullException(«token»); — честно говоря, не знаю, а нужно ли это? Нельзя ли просто сделать этот аттрибут как-то как Required и просто пользователь будет получать 404, либо самим кидать 404, либо не кидать ошибки, но выводить нормальные текст на странице о том, что ссылка не верна и почему?
в) Именования свойств LoginzaHelper не подходит стандартным именованиям, которые применяются в языках ООП.
г) ну код нужно причесать, типа вынести Convert.ToInt32(ConfigurationManager.AppSettings[@«WidgetId»]) нормально в нормальные методы или свойства
а) catch { return View(«Error»); } — нужно менять, например, на throw new HttpException(500);, да и вообще не смысла перехватывать, это сделает сама инфраструктура MVC
б) if (token == null) throw new ArgumentNullException(«token»); — честно говоря, не знаю, а нужно ли это? Нельзя ли просто сделать этот аттрибут как-то как Required и просто пользователь будет получать 404, либо самим кидать 404, либо не кидать ошибки, но выводить нормальные текст на странице о том, что ссылка не верна и почему?
в) Именования свойств LoginzaHelper не подходит стандартным именованиям, которые применяются в языках ООП.
г) ну код нужно причесать, типа вынести Convert.ToInt32(ConfigurationManager.AppSettings[@«WidgetId»]) нормально в нормальные методы или свойства
про (в) понял, что там dynamic, но ниже сказали — не кошерно это.
Да это пример кода :)
а) перехватывает, но если нужно вывести свою страницу, то логичнее обрабатывать exception
б) рефлектор поставил по команде Clean code :)
в) Какие? исправлю, вроде все по кодестайл
г) это я просто засунул для примера, что бы не городить лишнего.
а) перехватывает, но если нужно вывести свою страницу, то логичнее обрабатывать exception
б) рефлектор поставил по команде Clean code :)
в) Какие? исправлю, вроде все по кодестайл
г) это я просто засунул для примера, что бы не городить лишнего.
а) Мне кажется, у вас слабое представление о ASP.NET, ну и в частности об ASP.NET MVC. Если хочется вывести свою страницу, то это делается через customErrors.
б) Ну рефлектор не может быть правильным всегда :) Просто в этом случае ошибка ArgumentNullException не нужно никому, ни вам, ни пользователю. Вам в этом случае только нужно знать, что кто-то средиректил на такую страницу, потому страницы Error с 404 с головой хватит.
в) Я понял, что у вас там используется dynamic, вам уже внизу посоветовали, что лучше бы с оберткой сделать. Чтобы в случае изменения API — на это было бы более очевидное реагирование.
г) Примеры должны быть всегда лучше всего остального. Так как этот код прочитают больше народу, чем если бы это был бы просто код. Более того, молодое поколение будет просто копировать этот код и он пойдет в продакшн ;) Все мы люди немного ленивые, мне тоже часто перепадает критика за плохой код в примерах, но я понимаю, что не просто так.
б) Ну рефлектор не может быть правильным всегда :) Просто в этом случае ошибка ArgumentNullException не нужно никому, ни вам, ни пользователю. Вам в этом случае только нужно знать, что кто-то средиректил на такую страницу, потому страницы Error с 404 с головой хватит.
в) Я понял, что у вас там используется dynamic, вам уже внизу посоветовали, что лучше бы с оберткой сделать. Чтобы в случае изменения API — на это было бы более очевидное реагирование.
г) Примеры должны быть всегда лучше всего остального. Так как этот код прочитают больше народу, чем если бы это был бы просто код. Более того, молодое поколение будет просто копировать этот код и он пойдет в продакшн ;) Все мы люди немного ленивые, мне тоже часто перепадает критика за плохой код в примерах, но я понимаю, что не просто так.
Давайте я сюда отвечу
а) если нужно вывести свою страницу, то её и надо прописать в error redirects, а не заниматься порнографией в коде контроллера
б) вы головой программируйте, а не рефлектором
в) Анализ error_type на != null, а потом выброс error_message в прикладном коде выглядит жутким костылем.
Название метода «AuthInfo» само по себе охренительно, хуже только название Helper. Про dynamic вам уже сказали: сервис что-то вернет, а что — хрен его знает.
а) если нужно вывести свою страницу, то её и надо прописать в error redirects, а не заниматься порнографией в коде контроллера
б) вы головой программируйте, а не рефлектором
в) Анализ error_type на != null, а потом выброс error_message в прикладном коде выглядит жутким костылем.
Название метода «AuthInfo» само по себе охренительно, хуже только название Helper. Про dynamic вам уже сказали: сервис что-то вернет, а что — хрен его знает.
braindamaged, научитесь корректно отвечать! Ваш ответ должен быть таким:
а) Если нужно вывести свою страницу, то её надо прописать в error redirects
б) Анализ error_type на != null, а потом выброс error_message в прикладном коде — плохая практика.
Плюс название метода «AuthInfo» — мало информативно.
Все остальное в сообщении: конфликтогены и оскорбления. На Хабре это не приветствуются.
а) Если нужно вывести свою страницу, то её надо прописать в error redirects
б) Анализ error_type на != null, а потом выброс error_message в прикладном коде — плохая практика.
Плюс название метода «AuthInfo» — мало информативно.
Все остальное в сообщении: конфликтогены и оскорбления. На Хабре это не приветствуются.
Динамические объекты — это тое же object'ы, но со специальным атрибутом. Можно возвращать object и потом рефлексией доставать из него необходимые поля.
Ну а вообще, в данном случае dynamic используется неоправдано. Названия полей изменяться не будут, поэтому логичней было бы написать класс-обертку для возвращаемого результата AuthInfo(). А так получается, что кроме геморроя для старых проектов, вы добавляете еще и геморрой с угадыванием названий возвращаемых полей.
Ну а вообще, в данном случае dynamic используется неоправдано. Названия полей изменяться не будут, поэтому логичней было бы написать класс-обертку для возвращаемого результата AuthInfo(). А так получается, что кроме геморроя для старых проектов, вы добавляете еще и геморрой с угадыванием названий возвращаемых полей.
Может на GitHub ее? Я бы покомитил.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
.NET библиотека для сервиса Loginza