The Good, the Bad and the Ugly code


    Хороший код или плохой? Лично для меня хороший код обладает следующими качествами:
    • Код легко понятен разработчикам разной квалификации и хорошо структурирован
    • Код легко изменять и поддерживать
    • Приложение выполняет свои функции и обладает достаточной, для выполняемого круга задач, отказоустойчивостью

    Несмотря на короткое описание, о том, как добиться выполнения трех этих условий, написано много толстых книг.

    Почему именно эти критерии? Сразу оговорюсь, речь сейчас идет о разработке ПО для бизнеса (enterprise application). Критерии оценки кода для систем реального времени, самолетов, систем жизнеобеспечения и МКС отличаются.


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

    Кейс №1


    На форме есть поле ввода текста (textarea). Можно ввести туда 100500 символов и наше приложение свалится с исключением «превышена максимальная длина запроса».

    Является ли это нашим кейсом? Едва ли. Если это публичная часть, стоит поставить Java-script валидатор и не давать постить такие формы. Настраивать как-то дополнительно сервер для тех, кто отключил java-script – просто трата времени, а значит – бюджета. Мы не будем этого делать. Да, в нашем приложении есть потенциальная ошибка. Нужно ли кому-то, чтобы ее исправили? Нет.

    Кейс №2


    От клиента приходит запрос: нам нужна страница партнеров. На этой странице будет краткая информация о каждом из них и ссылка на страницу, на которой мы должны встроить в iframe страницу с сайта партнера.

    Интересный кейс. Наверное, нам нужно внести изменение в БД, добавить сущность партнер, db-entity by design мы не передаем во View, так что нужна еще DTO, функция маппинга из db-entity в dto, еще миграция БД для авто-деплоя. Хм, еще нужна страница в админке, чтобы добавить партнера. Не забыть о роутинге для контроллера, чтобы было /Partner/<PARTNER_NAME> Довольно много работенки…

    Верно для 10.000 партнеров. Давайте поинтересуемся: «сколько партнеров будет»? Если ответ «в этом году три». Нам стоит пересмотреть свои взгляды. Это опять перерасход времени и бюджета. Всего 3 партнера?


    namespace Habrahabr.Models.Partner
    {
        public class PartnerViewModel
        {
            public string Name { get; set; }
    
            public string WebSiteUrl { get; set; }
    
            public string ShortDescription { get; set; }
    
            public string FullDescription { get; set; }
        }
    }
    
    using System.Web.Mvc;
    using Habrahabr.Models.Partner;
    
    namespace Habrahabr.Controllers
    {
        public class PartnerController : Controller
        {
            private static PartnerViewModel[] partners = new[]
    	 {
    		 new PartnerViewModel()
    			 {
    				 Name = "Coca-Cola",
    				 WebSiteUrl = "http://coca-cola.com/partner",
    				 ShortDescription = "Coca-cola short description", 
    				 FullDescription = "Coca-cola full description"
    			 },
    		 new PartnerViewModel()
    			 {
    				 Name = "Ikea",
    				 WebSiteUrl = "http://ikea.com/partner",
    				 ShortDescription = "Ikea short description", 
    				 FullDescription = "Ikea full description"
    			 },
    		 new PartnerViewModel()
    			 {
    				 Name = "Yandex",
    				 WebSiteUrl = "http://yandex.ru/partner",
    				 ShortDescription = "Yandex short description", 
    				 FullDescription = "Yandex full description"
    			 }
    	 };
    
            public ActionResult Index()
            {
                // TODO: populate partners from database if a lot of partner required
                return View(partners);
            }
            
            // TODO: refactor this if a lot of partner required
            [ActionName("Coca-Cola")]
            public ActionResult CocaCola()
            {
                return View("_PartnerDetail", partners[0]);
            }
    
            public ActionResult Ikea()
            {
                return View("_PartnerDetail", partners[1]);
            }
    
            public ActionResult Yandex()
            {
                return View("_PartnerDetail", partners[2]);
            }
        }
    }
    
    @model IEnumerable<Habrahabr.Models.Partner.PartnerViewModel>
    
    <ul>
        @foreach (var p in @Model)
        {
            <li>
                <h3>@Html.ActionLink(p.Name, p.Name, "Partner")</h3>
                <div>@p.ShortDescription</div>
                <p class="info">@Html.ActionLink("Partner page", p.Name, "Partner")</p>
            </li>
        }
    </ul>
    
    @model Habrahabr.Models.Partner.PartnerViewModel
    <h2>@Model.Name</h2>
    <div>@Model.FullDescription</div>
    @if (!string.IsNullOrEmpty(Model.WebSiteUrl))
    {
        <iframe width="100%" height="500" src="@Model.WebSiteUrl"></iframe>
    }
    




    На все про все – максимум час времени с учетом ввода текстов. Мы разработали достаточно хорошую страницу партнеров. Отвечает ли он нашим критериям:
    • Понятен и структурирован – да
    • Легко изменяем – да
    • Достаточно надежен – да

    Предметная область, доступ к данным и архитектура приложения


    Фаулер правильно заметил в своей книге Enterprise Patterns of Enterprise Application Architecture, что термин «архитектура» невероятно размыт:
    Термин «архитектура» пытаются трактовать все, кому не лень, и всяк на свой лад. Впрочем, можно назвать два общих варианта. Первый связан с разделением системы на наиболее крупные составные части; во втором случае имеются в виду некие конструктивные решения, которые после их принятия с трудом поддаются изменению. Также растет понимание того, что существует более одного способа описания архитектуры и степень важности каждого из них меняется в продолжение жизненного цикла системы.

    У выше описанного подхода с партнерами есть одно существенное ограничение. Нельзя постоянно писать код приложения в таком стиле. Рано или поздно система вырастет. В данном случае мы «зашили» логику приложения. Еще пара таких контроллеров и понять работу системы будет довольно сложно, особенно новым разработчикам.
    В момент, когда выяснится, что партнеров будет не 3, а 100500 и показывать надо не всех, а только тех, кто заплатил в ночь с пятницы на субботу (с 13 на 14ое) и принес в жертву 6 девственниц, а страницы надо ротировать в соответствие с положением Венеры относительно Юпитера (в реальной жизни кейсы, конечно, другие, но не зря говорят, что для разработчика нет ничего более нелогичного, чем «бизнес-логика», состоящая сплошь из исключений и частных случаев). Нужно задуматься об архитектуре в обоих смыслах слова.

    Мы заблаговременно оставили «швы» в приложении, начнем распарывать их и пришивать необходимые куски. Мы воспользуемся ORM, переименуем PartnerViewModel в Partner и замапим класс партнера в БД. Я никогда не использовал Database-First и не воспринимал всерьёз Entity Framework, пока не вышла версия с Code-First подходом. В данном случае, я не вижу необходимости мапить Partner в PartnerViewModel. Да, семантически эти сущности выполняют разные задачи и в общем случае PartnerViewModel может отличаться, но пока нет ни единой причины писать абсолютно такой-же класс. Мапить ViewModel имеет смысл, когда без этого нельзя обойтись, не потащив логику в представление. Обычно необходимость во ViewModel’ях возникает, если в интерфейсе требуется показать сложную форму, оперирующую сразу несколькими сущностями предметной области, или есть необходимость воспользоваться атрибутами из сборки System.Web для свойств модели. В последнем случае, я склонен тащить за собой эту зависимость в сборку с предметной областью, если я уверен, что в ближайшие полгода в приложении не появится новой точки входа, кроме основного веб-приложения.

    В одном проекте, в котором я участвовал, для доступа к данным необходимо было вызвать сервис, который обращался к фасаду, который в свою очередь обращался к DAL-слою, который вызывал Entity Framework. Фасад и DAL находились в разных сборках, а EF использовал подход Database-First. Всю логику обычно выполнял DAL, а остальные слои просто перемапливали результаты в 90% случаев в абсолютно идентичные DTO. При этом никаких других клиентов у системы не было и сервис был объективно не нужен. Когда я спросил «зачем такой оверхед», разработчик, который дольше работал на проекте сказал: «Не знаю, наш другой проект так написан, решили сделать по аналогии». Да, этот «другой проект» состоял из 20 приложений и только на то, чтобы развернуть его ушло 2 дня. У API этого приложения много клиентов. Такая сложная структура была необходимостью. В нашем случае это была стрельба из пушки по воробьям.

    Но вернемся к нашим баранам партнерам. Осталось понять, где написать код, отвечающий за выбор необходимых партнеров для показа. Напоминаю, что теперь мы должны учитывать Венеру и Юпитер. Худшее место – это контроллер. Поступив так мы разбросаем логику приложения по разным его частям и новым разработчикам придется собирать ее (логику), подобно Гарри Поттеру, собиравшему крестражи.
    Хороший вариант воспользоваться паттерном репозиторий спецификация.

    IOC/DI


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

    Не буду в очередной раз описывать, что такое IOC/DI и с чем его едят, просто нужно взять за правило:
    • Не создавать зависимости явно
    • Использовать IOC-контейнеры

    При этом не обязательно создавать интерфейс на каждый класс в проекте. Зачастую интерфейс бывает проще выделить в дальнейшем. R# прекрасно справляется с этой задачей. Но держите это в голове, когда будете писать класс. Мыслите его интерфейсом, а не конкретной реализацией. Используя DI вы сможете гибко управлять реализациями. В случае необходимости, вы сможете выкинуть одну реализацию и заменить ее другой, например, если по каким-то причинам база данных перестанет вас устраивать, вы сможете прозрачно заменить ее, заменив реализацию репозитория. Кроме этого, используя IOC/DI гораздо проще писать тестируемый код.

    Тесты


    Я не фанат 90-100% покрытия проекта. Я думаю, что чаще всего, это замедляет разработку и делает поддержку кода гораздо более муторным занятием. Я предпочитаю покрывать тестами поведение системы, т.е. основные бизнес-правила приложения. Таким образом, тесты не только страхуют от ошибок, но и помогают новым разработчикам быстрее понять логику работы системы. Такие тесты работают лучше, чем любая документация. Особенно эффективны тесты, использующие BDD – нотацию.

    Дальнейшее развитие системы


    Если приложение продолжает расти, сложно дать конкретные советы: слишком много факторов, которые необходимо учесть. Слишком многое зависит от специфики проекта. Хорошим решением может быть SOA. Вообще декомпозиция любой большой системы – отличная идея. Десять проектов загружено у вас в IDE или сто — разница очень большая. При разработке софта такого масштаба не обойтись без deploy-скриптов, инсталляторов и релиз-менеджмента, но это уже совсем другая история…

    Лямбды, аспекты, функциональщина и прочий холивор


    Напоследок решил оставить заведомо спорные моменты. Я заметил, что многие разработчики склонны «привыкать» к стеку, с которым они работают. Если программист «привык» к своему стеку, он склонен, тащить его в каждый новый проект, независимо от целесообразности. Так, я неоднократно слышал критику C# за «заигрывания» с функциональным стилем, лямбды, которые компилируются в «черт-знает что» и «не отлаживаются» и за прочие extension-методы. Объективно, мы видим, что в php добавили замыкания, в java8 появится Stream для потоковой обработки коллекций, значит многим разработчикам это по душе.

    Не стоит бояться комбинировать парадигмы, если оно того стоит. Задайте себе вопросы «на сколько платформа/язык/ос подходят для выполнения задачи?» и «какие технологии помогут мне выполнить задачу максимально эффективно?». Лучше всего мою точку зрения иллюстрирует цитата iliakan:
    Я так прикинул, что будет быстрее выучить Erlang или заставить Java работать достаточно быстро и решил выучить Erlang

    Книги на тему


    Поделиться публикацией
    Похожие публикации
    AdBlock похитил этот баннер, но баннеры не зубы — отрастут

    Подробнее
    Реклама
    Комментарии 18
    • 0
      Что-то я не очень понял на счет треугольника. Разве время и деньги не связаны воедино?
      Я бы даже сказал что они все связанны вот так: качество = стоимость / время
      • +9
        Треугольник параметров — это же эталонное «Быстро, качественно, недорого. Выбирайте любые два».
        • +4
          По вашей формуле получается, что дешево, но очень быстро — это качественно.
          • –4
            Ошибаетесь. По этому треугольнику получается, что если быстро и дешево, то качества не будет.
            • 0
              Упс. Это я ошибся, так как речь не о треугольнике, а о формуле в первом посте.
            • 0
              Чем дальше точка от параметра, тем меньшее воздействие он на эту точку оказывает.
              • 0
                Просто качество — лишь одна сторона результата. Согласитесь, что быстро невозможно создать что-то полноценное, получается просто абсурдно, если подставить маленькие цифры.
                Каждый сам для себя может умножить время на «коэфицент раздолбайства хаброчитайства»
            • +15
              Ужасный подход. Если на каждое требование заказчика «хреначить говнокод», чтобы «дешевле» — здесь никакой архитектуры (и по Фаулеру и без него) быть не может. Учитывая, что впоследствие на переделку денег также обычно никто давать не хочет. Сама архитектура приложения должна быть гибкая, чтобы просьба «добавить страницу партнеров» не вызывала у программистов чувство страха.

              Недавно проскочило во френдленте:
              image
              • +4
                Я не вижу в статье примеров говнокода. Посыл в ином. Нет смысла вводить новую таблицу и делать админку, если в ближайшей перспективе будет добавлено всего 3 партнера, потому что:
                • Возможно это функционал вообще «не пойдет» и по окончанию контракта нужно будет удалить эти разделы к чертовой бабушке
                • Требования могут поменяться кардинально — меньше кода — значит меньше придется переписывать/рефакторить
                • Это время можно потратить на какой-либо другой более важный функционал
                • <Во второй части я сам пишу, что так нельзя делать бесконечно и в определенный момент нужно сделать рефакторинг… В данном случае рефаторинг сделать достаточно просто.

                Более того ваш тезис «архитектура должна быть гибкой» не противоречит тому что я написал. Я пишу, что нужно создавать «легко-изменяемый» код. Думаю, что это синоним гибкости. Можете аргументировать, что тут «ужасного» и где здесь «говнокод»?
                А то что денег не дадут, если вы не можете обосновать необходимость рефакторинга для бизнеса:
                • Стоит научиться говорить с клиентом на одном языке
                • Может быть это действительно рефакторинг ради рефакторинга?

                Нужно понимать, если вы не пишете «нефте-газо-добывающий» софт, деньги у клиента не растут на дереве и от успешности его бизенса зависит его возможность платить вам. Программистам свойственно обращать внимание только на код: то, что большинство ваших пользователей и заказчиков даже не может «потрогать руками». Код — это не цель, а средство для приложения. Плохо-структурированный сильно-связанный код содержит ошибки и его сложно расширять, поэтому такие средства не дают достижения цели, но кроме кода есть еще бюджет, юзер-интерфейс, маркетинг, дистрибуция. Попробуйте с нуля разработать и продать свое приложение. Когда ваши доходы будут напрямую зависеть от продаж продукта, а не измеряться в ежемесячной величине.
                • +3
                  Все правильно автор говорит. Великий опыт хорошего разработчика и состоит с том, чтобы не лепить горы абстракций там, где не надо, и четко знать, где можно «сговнокодить».

                  Например, обучая джуниора основам С, нельзя ему даже рассказывать про оператор goto. Опытный же программист знает, как его правильно применить при, скажем, выходе из циклов большой вложенности. Точно также, иногда можно и сговнокодить, если ты хорошо понимаешь что ты делаешь, и если это не скажется на архитектуре всей системы в целом.
              • –4
                С ростом приложения возникает опасность сделать его слишком сильно-связанным, а это сделает систему неповоротливой и будет мешать изменению кода.

                Не буду в очередной раз описывать, что такое IOC/DI и с чем его едят, просто нужно взять за правило:
                Не создавать зависимости явно
                Использовать IOC-контейнеры

                Явное лучше чем не явное.
                IOC-контейнеры не облегчают жизнь, лишь позволяют делать конфигурируемыми некоторые элементы. Втыкать их везде приложение — зло.
                • 0
                  Вообще зависимости лучше задавать через интерфейсы. Но я бы не называл это неявным.
                • +1
                  А по поводу тестов можно ломать много копий, но чтобы не плодить их слишком много проще всего использовать следующее правило: любое нетривиальное изменение кода должно приводить к тому, что ломается как минимум один и, в идеале, ровно один тест.

                  Разумеется всегда можно изменить код так, чтобы тесты не порушились, а код перестал работать (можно, например, отдельно обработать варианты, используемые в тестах), но мы же тут не с врагами, а с ошибками боремся. Другая крайность встречается реже, но мне с ней тоже довелось встретится и могу сказать, что току от неё тоже нет: если у вас добавление одного пробела в формат вывода приводит к тому, что падают 100500 тестов, то можете считать, что тестов у вас нет ибо нормальный человек не будет даже разбираться с тем, что произошло, а просто «одним махом» заткнёт их все и даже смотреть на причину их падения особо не будет.
                  • 0
                    Если упало много тестов и это невозможно проанализировать — плохо организовано тестирование.
                    Отличный пример толкового тестирования — SQLite. Да, система изначально писалась для военных целей, но никто не мешает перенять методику. Затратно, но действительно круто.
                  • +1
                    Тема затронута достаточно глубокая и интересная. Но, пример кода: короткий, красивый и бессмысленный.
                    Если на то пошло, то почему бы не сверстать эти же странички в Фронтпейдже, ведь их всего три, и в этом году не будут меняться?
                    • 0
                      Это пример из реальной жизни. Я переписал код, так как не вправе открывать код приложения. И реальный пример обсуждения. Мой коллега был за то, чтобы «сделать по-правильному» с БД, новой сущностью и т.д. На тот момент уже существовала инфраструктура. Если верстать на фронтпейдже мы будем вынуждены копипастить верстку в 3 файла. В приложении уже есть «нарезанный» лейаут, грех им не воспользоваться. Весь слой представления у нас уже готов, в крайнем случае потребуется добавить пагинацию. Это легко решается добавлением чего-то типа PagedList. Код в представлении останется таким же. А снизу добавится пагинатор. Список, объявленный в коде, гораздо проще отрефакторить в тот или иной персистанс (все поля уже объявлены), чем собирать сущность из трех html-страниц.

                      Большой пласт, который я не рассмотрел в статье — деплоймент. Суммарную сложность решения нужно рассчитывать в том числе, учитывая этот факт. Например, нужно добавить новый сервис. Можно реализовать его, как компонент, для существующего контейнера ил как самостоятельную службу/демон или захостить на веб-сервере. Возможно, что реализовать отдельную службу проще, но при этом придется менять процесс деплоймента. Но это уже отдельная тема. Автоматический деплой в основном реализуют в основном только «большие» компании/продукты. Пока большая часть разработчиков деплоит «руками» разговаривать об этом просто бесполезно. Из более простых примеров — главная страница приложения. Нужно показывать набор иконок. Можно захардкодить их во вью, а можно сделать запрос к БД. Если набор иконок меняется только с релизами, хранить их во вью проще.

                      Второй момент: логика, «размазанная» по приложению. Один функционал на уровне Entity, другой — в сервисах, третий — в котнтроллерах. Это, пожалуй, самый сложный момент, который относится больше не к «разработке» как таковой, а скорее к процессуальной составляющей. Все достаточно большие приложения, в разработке которых я принимал участие, так или иначе страдали от этой проблемы. Причем это опыт как на пост-советском пространстве, так и на европейском. Я думаю, что вообще нет серебряной пули, позволяющей избавиться от этой проблемы. Я обычно прошу всех разработиков, как можно раньше сообщать о сложностях связанных именно с дизайном системы, чтобы по возможности быстро провести рефакторинг, таким образом иссекая загнивающие куски в зародыше.
                      • 0
                        Второй момент: логика, «размазанная» по приложению. Один функционал на уровне Entity, другой — в сервисах, третий — в котнтроллерах. Это, пожалуй, самый сложный момент, который относится больше не к «разработке» как таковой, а скорее к процессуальной составляющей.

                        Мы решали это путем введеления кретериев которые бы указывали в какой слой выводить эту логику и выделяли в документ «Руководство разработчика», в котором велся changelog.
                        • 0
                          Да, это понятно, поэтому я пишу, что это скорее процессуальный момент.

                    Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.