Советы новичкам при проектировании модульных производственных систем

В этой статье я попытаюсь поделиться своим опытом в проектировании пользовательской бизнес-логики. Это явно не претендует на полноценный ликбез, т.к. я всего лишь вспоминаю то, через что прошёл лично я, какие ошибки я допустил, и как мне их удалось (или не удалось) исправить в будущем. Наверняка, опытные системные архитекторы уже все проходили и знают, однако надеюсь, что некоторые советы таки будут полезны.
Мы использовали (и используем) клиентскую часть на WPF/Silverlight, WCF сервисы и СУБД Oracle, Postrges, MsSQL. Код написан по MVVM, использована Prism для модульности и навигации. Не могу точно сказать, какие из тезисов подойдут для других платформ и языков.

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

Итак, поехали.

1. Стремление избавиться от повторения кода должно быть разумным


Получилось, что в двух разных сборках (говоря об интерфейсе – в двух разных модулях системы, которые вызываются в разные моменты времени) нужно было выбирать из БД похожий список сущностей, скажем методом GetComissions(). Программист Петя, увидев у программиста Васи уже написанный на сервисе метод GetComissions() (а у нас тогда был один сервис с одним эндпоинтом), недолго думая, взял и его использовал. Все бы хорошо, только через пару недель заказчику понадобилось в одном из мест кроме комиссий показывать еще и их статистику, статусы, решения, и много всего другого. Как результат, второй модуль сразу же стал падать с ошибкой.
Увы, но таких случаев было явно больше одного, и в последствии, понадобилось очень много усилий по разнесению методов сервиса на разные сервисы-эндпоинты, или хотя бы на отдельные партиал классы (в случае с одним сервисом).
Вывод: два проекта могут использовать одну и ту же функцию на сервисе тогда и только тогда, когда заранее заведомо известно, что эти проекты всегда будут работать с этой функцией одинаково. В таком случае метод должен выноситься в общий класс, который и подключается к каждому проекту.
Во всех остальных случаях принцип написания кода на сервере должен следовать чему-то похожему на Single Responsibility principle (один из принципов SOLID).

2. Старайтесь не писать логику в датаконтрактах


По моему опыту, абсолютно всегда решение проще, если все датаконтракты (в случае с RiaServices – энтити классы) остаются всегда девственно чистыми, и совпадают 1к1 со схемой БД. Всё может начаться с безобидного кода конкатенации ФИО работника в одну переменную, а заканчивается это огромными вычислениями каких-то непонятных коэффициентов (нужных, как правило, только в одном месте). В итоге датаконтракты стали на 80% состоять либо из методов (да, если в геттере свойства написан какой-то код, это тоже я считаю методом), либо из полей других таблиц, которые нужны были кому-то одному в какой-то момент. Всегда лучше делать наследники или обёртки на сервисе или на клиенте (в зависимости от задачи), которые будут использоваться сугубо для целей этого модуля. Про проблемы с ними – в следующем пункте.

3. Что лучше на клиенте – наследник, партиал?


Увы, но идеального решения мы так и не нашли. Рассмотрим несколько случаев:

а) Модули работают с одним клиентским референсом на сервис. Классы на сервере разбиты на партиал классы от одного сервиса.
Минусы:
  • Клиентские партиалы, созданные для одного модуля, видны везде.
  • Клиентские наследники, созданные для одного модуля, видны везде (с ними попроще – их можно не использовать где не нужно)
  • Геморрой с настройкой параметров copy local для референсов проекта с клиентским сервисом (при работе с MEF и необходимости иметь одну сущность какого-то класса в сервис локаторе).
  • Геморрой с мерджем сервисов в свне, при одновременной перегенерации двумя людьми.

Плюсы:
  • возможность передачи объектов датаконтрактов между клиентскими модулями.



б) Каждый модуль самостоятельно делает клиентский референс на сервис.
Минусы:
  • выбор между возможностью передачи датаконтрактов между клиентскими модулями (в таком случае создаётся проект клиентских датаконтрактов, см. рис) и возможностью создания клиентских наследников и партиалов (в варианте на рис. это очень неудобно)


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


Допустим, у нас в БД есть таблички Person и Document. По канонам, в датаконтрактах у класса Person будет List, т.к. у Document есть вторичный ключ на Person. Одной из задач может быть создание на клиенте сущностей Person и всех его Documents сразу, и последующее сохранение этого всего добра в БД. В этом нет никаких проблем, если использовать класс Person и его поле List.
Однако, если в таком случае, нам потребуется помимо создания отображать и информационные поля из других таблиц (статусы, количества других сущностей, связанных вторичными ключами с Person
или Document), и мы для этого сделаем наследника PersonEx и DocumentEx, то у PersonEx уже не будет поля List! В таком случае приходится писать кучу обёрток и переприсваиваний, что усложняет код – ведь в настоящей задаче уровней вложений может быть не два, а гораздо больше. В таких случаях нам помогут партиал-классы, однако нужно бдить, чтобы случайно эти же поля не заполнил кто-то другой в другом месте, т.к. партиалы видны везде.

4. Никогда не связывайте колонки, созданные для базовых нужд, и бизнес-логику


Под этими колонками я имею, прежде всего, идентификаторы. Связывание с бизнес логикой означает сортировки, группировки, использования идентификаторов в каких-то функциях, отображение их в интерфейсе, или еще хуже – функционал их редактирования напрямую.
Наверняка, это холиварная тема – но на моей практике еще не было случая, когда заказчик требовал бы какую-то сортировку, и для ее осуществления не было бы подходящего поля, заполняемого отдельно (руками или триггерами в БД – уже неважно).
При наличии связи идентификаторов с бизнес-логикой любой переход на другую схему БД, или на другую СУБД в принципе, или любое масштабирование системы на несколько БД с последующей репликацией повлечёт за собой переписывание логики.
Hint: очень удобно на каждую табличку в БД создавать две колонки date_insert
и date_update, в которые триггерами на те же инсерт-апдейт заполнять время этих операций.

5. Избегайте неявных инсертов в БД, не связанных с осознанным действием пользователя


Речь идет, конечно же, о создании сущностей бизнес-логики, а не о логгировании каких-либо действий. Я лично сталкивался с сопровождением системы, в которой запись о пропуске появлялась сразу же после печати его на принтере. Это была большая головная боль, т.к.:
  • Пришлось писать говнокод и передавать колбеки в модуль печати, который по-хорошему не должен знать вообще ни о чём.
  • Из-за плохого качества принтеров, драйверов и их обслуживаний, достаточно часто ивент о законченности печати приходил, данные вставлялись, а печать не происходила.
  • Перепечатывание стало мукой, т.к. приходилось править БД.

Идеальный вариант был сделан чуть позже – появилась кнопка «сохранить пропуск в БД», а первая кнопка осталась простым предпросмотром с возможностью печати бесконечное кол-во раз.
К тому же, предварительное сохранение данных в БД не кнопкой «сохранить» чревато очень быстрым засорением базы пустыми (лишними) записями, которые кто-то потом явно забудет (или не сможет) удалить.

6. Полностью ограничьте любую возможность присутствия в БД неверных данных


Рассмотрим для примера случай на рисунке.

Допустим, мы имеем дело с поставками какой-то нумерованной сущности в больших партиях в разные регионы (в данном случае - бланк). У него есть номер и серия, печатная компания печатает ParentDiapason, а потом подрядчик делит его на ChildDiapason-ы и распределяет по регионам.
Однако такая схема еще далека до идеала, т.к. в БД сможет храниться бланк с серией ПП и номером 245 в чайлд диапазоне с сериями от АА до КК и номерами от 300 до 400. Кроме этого, поле quantity наверняка зависит от границ диапазонов, поэтому БД должна запрещать наличие записи, у которой quantity будет неравна результату некой функции от (startSeries, endSeries, startNumber, endNumber). Таким образом, мы приходим к списку триггеров:
  • на инсерт и апдейт бланка:
    • его серия и номер должны входить в чайлд диапазон
  • на инсерт и апдейт чайлд диапазона:
    • его множество серий и номеров должно быть подмножестом парента
    • не пересекаться с другими чайлдами
    • все смотрящие на него бланки должны быть в его множестве серий и номеров
    • поле квонтити должно совпадать с размером, вычисляемым по сериям и номерам
  • на инсерт и апдейт парент диапазона – аналогично с чайлдом.

Только после включения этих констреинтов с такой схемой можно работать в реальной системе.
Hint: при работе с MsSQL Server 2012 можно перенести класс-проверяльщик правильности данных в отдельную сборку, и ее использовать как на веб-сервере, так и в триггерах БД.

7. Используйте статусы сущностей вместе с конвертерами для интерфейса


Наверняка прозвучит по-капитански совет не иметь в коде никаких цифр, а выносить их в константы. Этот пункт немножко развивает этот совет, предлагая ипользовать удобное хранилище статусов сущностей:
  1. Создаётся класс, где над каждым статусом помечается атрибутом его текстовое значение (родительский класс – название таблицы, вложенный – название колонки)
    public class Users
    {
        public class Status : StatusBase<Status>
        {
            /// <summary>
            /// Активен (может логиниться)
            /// </summary>
            [StringStatus("Активный")]
            public const int Active = 1;
    
            /// <summary>
            /// Неактивен (не может логиниться)
            /// </summary>
            [StringStatus("Неактивный")]
            public const int Inactive = 0;
        }
    }
    

  2. Класс StatusBase – дженерик, создающий статический словарь (значение-объяснение), заполняющий его с помощью рефлексии
    public class StatusBase<T> where T : class
    {      
        private static Dictionary<int, string> statusesDictionary;
    
        public static string GetStringStatus(int key)
        {
            if (statusesDictionary == null)
            {
                CreateStatusDictionary();
            }
    
            string result = statusesDictionary.ContainsKey(key) ? statusesDictionary[key] : "Неизвестный статус";
            return result;
        }
    
        private static void CreateStatusDictionary()
        {
            statusesDictionary = new Dictionary<int, string>();
            FieldInfo[] fields = typeof(T).GetFields();
            foreach (FieldInfo field in fields)
            {
                string TextStatus = ((StringStatusAttribute)field.GetCustomAttributes(typeof(StringStatusAttribute), false)[0]). TextStatus;
                int statusKey = (int)field.GetValue(null);
                statusesDictionary.Add(statusKey, TextStatus);
            }
        }
    }
    

  3. Теперь, в любом конвертере, который будет в интерфейсе приводить цифровое значение в текстовое, достаточно просто вызвать метод
    public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
    {
        string result = Data.Statuses.Users.Status.GetStringStatus((int)value);
        return result;
    }
    


Таким образом, и значения статусов, и их текстовые объяснения хранятся в одном месте, и легко меняются при необходимости.
Ну вот и всё. Буду рад, если эти советы кому-то принесут пользу.

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

    +1
    GetStringStatus не thread-safe в этом месте:
    if (statusesDictionary == null)
            {
                CreateStatusDictionary();
            }

    Можно обернуть проверку, что словарь не создан, в lock(something) {… }, но будем выполнять лишний lock при каждом вызове GetStringStatus.
    Оптимально генерацию словаря засунуть в static constructor. Компилятор сам позаботится о потокобезопасности и об оптимизации блокировок.
      +2
      да, вы правы конечно.
      но, мне кажется, конвертеры должны вызываться из потока UI, а он в сильверлайте один (в нашем случае).
      но это конечно не причина оставлять дырки в базовых классах.
      хотел показать прежде всего подход, не обратил внимания на недочёты реализации.
        +1
        На самом деле для таких вещей можно просто использовать Lazy, или же если вручную то в этом случае стоит лишь немного изменить код и он станет уже потокобезопасным (т.к. тут нам неважно ни количество инициализаций, ни уникальность публикуемого словаря — никакие блокировки не нужны):

        private static void CreateStatusDictionary()
        {
            var sd = new Dictionary<int, string>(); // создали локальный словарь, его видим только мы
            FieldInfo[] fields = typeof(T).GetFields();
            foreach (FieldInfo field in fields)
            {
                string TextStatus = ((StringStatusAttribute)field.GetCustomAttributes(typeof(StringStatusAttribute), false)[0]). TextStatus;
                int statusKey = (int)field.GetValue(null);
                sd.Add(statusKey, TextStatus); // заполнили
            }
            statusesDictionary = sd; // публикация
        }
        

        0
        Оптимально использовать примитив Lazy для этой цели.
          +1
          Можно же сделать double-check lock, тогда lock будет вызван только при первых нескольких вызовах (в идеале — только при первом вызове)

          if (statusesDictionary == null)
          {
          lock(syncobject)
          if (statusesDictionary == null)
          CreateStatusDictionary();
          }

          Почему-то форматирование текста не работает :(
          –3
          Почему множество, а не сэт, с помощью, а не с хэлпом, включения, а не инклюда? etc.
            0
            «Пиплы! Коцайте тикеты! Некоцаный тикет за отмазу не канает!»… За что человека минусуете? Вполне адекватная претензия на переизбыток жаргонизмов в тексте.
            0
            Ещё пара вопросов:

            1. Если вы наследуете ExPerson от Person, то как создать объект-наследник, имея на руках объекта-предка? (просто копировать одноимённые поля как-то скучно, особенно когда их много).

            2. Entity-классы у вас передаются по WCF?
              0
              1. да, уже используем редко наследники, но приходится переприсваивать. думаю что должны быть какие-то более элегантные методы, но глубоко не копался.

              2. чаще блтулкит чем EF, пишем руками датаконтракты. передаются конечно, как иначе?
                0
                2. lazy-загрузка → проблемы при сериализации.
                Например, Document связан с Person, у Person ещё вагон документов. Так можно всю базу вытянуть при сериализации объекта, если ссылки хорошо прописаны. Кроме того, закрыл контекст (транзакцию) и lazy-поля, которые не прогружены, начинают кидать исключения при доступе.

                В-общем, много граблей и мы отказались от выдачи entity-объектов за пределы кода, который выполняется в транзакции.
                Для передачи по WCF отдельные классы (они же, по совместительству view models, хотя это тоже от лени).
                  –1
                  оо, я вспомнил этот гемор с EF — у нас были еще и сущности которые смотрели друг на друга =)
                  это кстати одна из причин почему блтулкит — указал явно какие поля тебе нужны, и вытянул.
                  в EF большой соблазн написать всю логику на клиенте и сделать SubmitChanges — тогда сервер превращается в какой-то транслятор данных =)
                    +1
                    Есть ещё причина не передавать entity с сервера наружу.
                    Каждому клиенту (особенно если предоставляешь WCF/SOAP API другой команде) желательно давать отдельный интерфейс с отдельными dto-объектами. Чтобы в случае, когда для кого-то надо поменять пару полей/методов, не порушить у других.
                      0
                      Под это даже специальная технология имеется: msdn.microsoft.com/ru-ru/library/cc668792.aspx
                  0
                  1.1. Создать конструктор (или статичный фабричный метод) у PersonEx с параметром Person. Там, конечно, придется копировать скучно, но может рефлексия вас развеселит :)

                  1.2. PersonEx и Person не наследник-родитель, а реализуют один интерфейс IPerson, но Person напрямую, а PersonEx как прокси к Person, который является приватным полем PersonEx (устанавливается аналогично 1.1, сеттер лучше не использовать — можно забыть установить)

                  1.3 Отказаться от наследования и перейти к агрегации :)
                  +1
                  Вывод: два проекта могут использовать одну и ту же функцию на сервисе тогда и только тогда, когда заранее заведомо известно, что эти проекты всегда будут работать с этой функцией одинаково.

                  То есть практически никогда. По-моему, нужно не только вовремя сливать две фунции в одну, но и вовремя разливать (возможно оставляя общую фунциональность в общей функции, то есть ни одна, ни две, а три функции будет). А чтобы легко определить это вовремя нужно писать тесты и не забывать их прогонять.
                    0
                    Тоже возник вопрос по этому пункту.
                    Если есть полноценное документирование, с отражением того, что такой-то класс стал зависим от такого-то, да еще и всё это прикрыто более-менее нормальным тестированием, то я абсолютно не вижу проблемы с такой зависимостью. У программиста ответственного за наследуемый класс (мы ведь его наследовали, правда?) возникла задача существенно его переработать, что может повредить зависимым классам. Он открыл описание класса и увидел там зависимый класс. Напомнил архитектору, что тут есть такая зависимость. В ТЗ внеслось изменение, родительский класс разделился на два — абстрактный, с общим функционалом, и конкретная реализация, которую уже можно править. Ну и в зависимом классе из соседней ветки изменили от кого наследуем. Ну или сами просто отнаследовались от изначального класса, и внесли в дочку изменения, чтобы не затрагивать соседей. (Мы ведь соблюдали LSP, правда?). Но тут уже от реализации зависит что будет проще.

                    ИМХО чрезмерная борьба с дублированием кода чревата не проблемами с зависимостями — это рабочий процесс, а перегруженностью пространства имен. Т.е. когда у тебя столько классов, что они есть на все случаи жизни, но когда будет нужно ты и не вспомнишь, и в документации не найдешь, что у тебя уже есть класс для этого, и 80% твоего времени уйдет на листание документации проекта.
                      0
                      немножко не так.
                      вы говорите о зависимых друг от друга классах, я же — о методах сервиса, вызывающихся из разных мест клиента. в моем случае невозможно сразу в один клик определить, где этот метод еще используется. и еще более невозможно определить, повлияет ли моё изменение на все другие его вхождения (или оно может быть там нужно, тоже?)

                      Он открыл описание класса и увидел там зависимый класс.

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

                      Ну и в зависимом классе из соседней ветки изменили от кого наследуем.

                      в моем случае такие изменения будут занимать 2-3 дня :) есть же классы, общие для всех проектов — в других солюшенах, есть классы, общие для модулей внутри одного проекта, и т.д.
                        +1
                        Администрирование зависимостей делается через теги @ uses & @ used-by или их аналоги в вашей среде разработки.

                        Что касается реального кода, то тут я тоже не вижу проблем.
                        Понадобился нам функционал, который уже реализован в FooView->render(), который разработан и поддерживается другим разработчиком.
                        За полминуты прикинул, что в принципе можно его оттуда заюзать. Прописал соответствующие теги комментирования в своем классе и используемом классе. В идеале только одну ссылку сделал, а вторая появилась автоматически.
                        На всякий случай о своем решении уведомил тикетом архитектора/ПМа или кто в проекте блюдет общую структуру.
                        Спустя надцать итераций рефакторинга у нас появился фичреквест требующий изменения поведения класса FooView в том месте где он изначально появился. Тут все просто — проводится стандартный рефакторинг — создаем класс BigFooView, пока пустой, наследник от FooView. Запускаем поиск-и-замена в своем любимом IDE с запросом использования класса FooView ограничив поиск в сфере собственной ответственности, и бегло просмотрев полученный список заменяем все это на BigFooView. Аналогично создаем класс SmallFooView на который заменяем использования нашего класса за пределами нашей сферы ответственности. Отмечаем FooView абстрактным (попутно переименовав его в соответствии с вашим стандартом кодирования для абстрактных классов). Пускаем тесты. Убеждаемся что все осталось целеньким. «Распиливаем» исходный класс вынося код который должен отличаться в дочерние классы (изначально естественно одинаковым, по возможности сохранив побольше общего кода в родительском классе используя или защищенные методы или вызовы родительских методов — согласно логике работы класса). Прогоняем тесты, убеждаемся что все ок, подправляем все док-комменты, вносим нужные изменения в BigFooView.

                        Если подобный рефакторинг занял больше чем 5-7 минут, то либо вы еще плохо дружите с IDE или не освоились в среде, либо изначальное решение об использовании чужого класса было очевидно ошибочным (что сомнительно), либо идея о внесении изменений в поведение столь активно используемого класса было явно ошибочным. Ваша оценка в 2-3 дня меня как-то уж очень смущает. Я за 3 дня в небольшом CRUD-проекте на пару сотен модулей (с таким же количеством моделей и т.п., с разными статистическими и join запросами и т.п.) общим весом чуть больше мегабайта кода — менял ORM. Т.е. довольно крупное АПИ заменялось, а не просто подмена одного класса. Если у вас действительно такие масштабы, то надо как-то обеспечивать обратную совместимость и т.п.

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

                  Самое читаемое