С утра вы приходите на работу, выпиваете чашечку кофе, закусывая печенькой, и в полной боевой готовности идете на свое рабочее место. Заходите в Jira (любой другой трекер), выбираете наиболее приоритетную задачу из текущего спринта и переводите ее в статус “In Progress”. Спустя некоторое время увлеченной работы задача отправляется на код-ревью, а вы позволяете себе отлучиться еще за одной чашкой кофе.


Вы довольны собой – к решению задачи вы подошли ответственно и копнули вглубь, проработав все возможные сценарии развития событий и написав красивый и лаконичный код. Вы возвращаетесь с кухни спустя несколько (десятков) минут, отвечаете на пару сообщений в одном из чатов и замечаете, что ваш почтовый ящик не пуст. Это пришла нотификация с гитхаба – ваш ревьюер оставил около 10 комментариев и сделал change request.



Знакомая ситуация?


Вы открываете свой Pull Request и видите все эти комментарии. Вы начинаете разбираться – первые 3-4 комментария требуют тривиальных правок, и вы начинаете думать, что на самом деле особых проблем в вашем коде нет. Пока не пролистываете change request до конца.


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


Однако, вы честно пытаетесь понять, почему он оставил такие комментарии, и копаете глубже. Само собой, у вас не должно быть никаких оснований для недовольства, если вы написали что-то вроде этого:


switch (object.getField()) {
    case "value_1":

        // Ага, мы сюда попали по value_1, значит это такой-то кейс

        if (condition_1 ||
                condition_2 ||
                condition_3 ||
                condition_4) {

            // some actions

        } else if (condition_5) {

            // another actions

        } else if (condition_6) {

            // another another actions

        } else if (another_fucking_condition) {

            // another fucking actions

        } else {
            log.warn("Let's just write this warning: {}", "some_explanation");
        }

        break;

    case "value_2":

        // Ну, value_2 говорит совсем о другом кейсе

        // some actions

        break;

    case "value_3":
    case "value_4":
    case "value_5":
    case "value_6":
        // do some common staff
        break;
}

Если вы пишете такое, то это говорит либо о том, что вы хреновый программист, либо о том, что вы просто не паритесь и не вкладываетесь в проект. Заработало – здорово. Тут у ревьюера есть все основания ткнуть вас носом в ваш же говнокод. Начав с совершенно идиотских кондиций вроде (condition_1 || … || condition_n), которые можно либо упростить, либо вынести в отдельный метод conditionsSatisfied() (нужно обдуманное название), и заканчивая тем, что для таких случаев здоровые люди используют фабрики. В итоге весь код сводится к следущему:


SomeFactory.get(object.getField()).execute(...);

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


Допустим, у вас есть два модуля – в одном вы ориентируетесь как рыба в воде, а в другом – как в лесу. Однако, у вас есть доступ к коду второго, и вы можете его править. Рассмотрим исходные условия этого примера:


Дано:


  • 2 модуля, один из которых шлет сообщение другому. Допустим, через REST запрос, хотя это не важно.
  • Есть сущность в базе данных, которая отслеживается в обоих модулях.
  • Модуль первый (ваш) в какой-то момент понимает, что объект больше не нужно отслеживать. Он шлет запрос во второй модуль и просит его сделать соответствующие изменения в базе.

Первые 2 пункта – чисто вводная, вся соль в третьем.


Вы пишете элегантный и красивый код. Вы не изобретаете велосипедов и используете именно ту http-библиотеку, которую принято использовать на вашем проекте (возможно, это даже не 3rd-party, а еще один внутренний компонент). Во втором модуле вы создаете отдельный package, который ответственен за общение именно с вашим модулем, а также создали роутинг. Покопавшись в коде модуля, вы с радостью находите метод, который называется весьма обнадеживающе – untrack_object(...). Вы смотрите тело метода и обнаруживаете, что это именно то, что вам нужно. В конце концов, все чертовски логично – если объект отслеживается, а следовательно, есть метод start_tracking(...), то должен быть и обратный сценарий. В итоге у вас получается что-то вроде этого (псевдокод):


@route('/resource/untrack/<id>', methods=[‘POST’])
@require_login
@another_annotation
def untrack_object(user, id):
    o = get_entity_by_id(id)
    Tracker.untrack_object(o)
    Entity.delete(o)

    notify_users_about_deletion(user, o)

    return jsonify(ok=True)

Вы пишете юнит-тест, тестируете руками – все работает. Объект больше не отслеживается и удаляется из базы, но ревьюер все равно с чем-то не согласен. А коммент его следующий: «Нельзя просто так взять и удалить объект. Необходимо пометить его как архивный, а не удалять из базы – он может еще пригодиться в будущем. А также необходимо произвести еще некоторые подготовительные действия». В комментарии он пишет вам опять-таки псевдокод:


def untrack_object(user, id):
    o = get_entity_by_id(id)
    Tracker.archive(o)
    o.archive()

    some_other_actions_with_o
    and_more

    notify_users_about_deletion(user, o)

    return jsonify(ok=True)

Вы понимаете, что он прав, но все равно негодуете: почему он вам пишет какой-то псевдокод, хотя самому ему написать рабочий код совершенно не составит труда? Он напишет это за пять минут, а у вас может уйти полчаса на то, чтобы разобраться и переделать.


Но делать нечего, и вы начинаете разбираться. Спустя полчаса работы вы:


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

Само собой, “ненависть” – лишь образное выражение, вы не должны и не будете ненавидеть его в прямом смысле. Однако, если время от времени у вас появляются негодования, когда вы видите очередной change-request с комментариями, вы должны понимать, что это только улучшит ваше знание системы и сделает вас более ценным разработчиком. Так что злитесь, “ненавидьте”, сходите в спортивную комнату и отлупите грушу, но затем успокойтесь и сделайте все как надо :)

Поделиться с друзьями
-->

Комментарии (54)


  1. Andrey_Volk
    28.05.2017 19:10
    +7

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


    1. samodum
      29.05.2017 02:34
      +31

      чтобы

      P.S. код-ревьюер


  1. MrShoor
    28.05.2017 21:10
    +45

    Начав с совершенно идиотских кондиций вроде (condition_1 || … || condition_n), которые можно либо упростить, либо вынести в отдельный метод conditionsSatisfied() (нужно обдуманное название), и заканчивая тем, что для таких случаев здоровые люди используют фабрики. В итоге весь код сводится к следущему:
    SomeFactory.get(object.getField()).execute(...);
    

    Не хочу обидеть, но я такое называю не здоровым программированием, а паттернами головного мозга.
    Было:
    Простое условие, вот прям в коде отлично видно что проверяется, и идем внутрь если true. Проще некуда.
    Предлагают:
    Фабрика… и вот мои дальнейшие действия чтобы потом разобраться что же там происходит:
    Заголовок спойлера
    Фабрика… о_О, в которую зачем-то передают только поле, а не сам объект… хм… значит фабрика хранит стейт? Стейт у неё скорее всего абстрактного типа, и где этот стейт устанавливается? хм… Идем дебагером внутрь execute(...), смотрим на приватные поля. Ищем методы в которых есть запись в эти приватные поля. Ставим брекпоинты, запускаем… а черт, кто то вызвал установку стейта раньше нашего кода… Ладно, начинаем искать где стейт устанавливается для нашего случая… искать по вызовам методов. Такс, кажется нашли, запускаем… и нифига, в брекпоинт не пришло. Эх блин, ну ладно… будем логировать в консоль, и отлаживаться через консоль.


    1. devalone
      28.05.2017 21:31
      +3

      Я в целом ничего против фабрик не имею, но те, кто пихают фабрики направо и налево — явно не понимают зачем они нужны.

      https://habrahabr.ru/post/141477/


      1. mavriq
        29.05.2017 00:54

        я абсолютно не приветствую создание везде и всюду универсальных фабрик фабрик фабрик.
        НО, последняя часть мне напомнила процесс, возможно, вам знакомый
        ДНК -> мРНК+рибосомы -> фолдинг белков («кромсание» «сырого» белка самим собой, выкусывание неиспользуемых блоков) -> последующая линковка нескольких белков в рабочие инструменты

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

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

        PPS
        ЛЛМ уже устарел. надо свою религию создавать. вот и база уже есть


    1. SirEdvin
      28.05.2017 21:48
      -2

      Фабрика с вероятностью 99.9% в данном случае плохо. Я в целом ничего против фабрик не имею, но те, кто пихают фабрики направо и налево — явно не понимают зачем они нужны.

      Конкретно случай, который обсуждается в статье — это получение объекта в зависимости от условий.


      И это практически всегда лучше засунуть в фабрику, если они у вас используются, потому что:


      • Далеко не всегда делаются проверки "а не создавал ли кто-то так же или почти так же объект в коде?"
      • Всегда проще будет поменять и найти логику.


      1. MrShoor
        28.05.2017 22:09
        +2

        Конкретно случай, который обсуждается в статье — это получение объекта в зависимости от условий.
        Ага, сделаем фабрику, чтобы сделать объект, чтобы этим объектом сделать execute, который проверит наши condition_1 || condition_2 || condition_3 || condition_4. Я как раз примерно об этом и говорю.


        1. SirEdvin
          28.05.2017 22:26
          -3

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


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


          1. MrShoor
            28.05.2017 22:40
            +1

            И в итоге у вас будет таких мест штук 6
            6 мест — не повод лепить фабрику с промежуточными объектами. Достаточно просто написать метод с входящими параметрами.


            1. SirEdvin
              28.05.2017 22:44

              И это будет фабричный метод. Не всегда, дурь написал.


              Автор просто из мира Java, а там сделать отдельно стоящий метод невозможно, следовательно, его нужно прилепить к фабрике, а не к одному из 6 мест в коде.


              Или я ошибаюсь?


              1. MrShoor
                28.05.2017 22:54

                Автор просто из мира Java, а там сделать отдельно стоящий метод невозможно
                Есть статические методы:
                public class Action {
                    public static void Execute(...) {
                    }
                }
                


                1. SirEdvin
                  28.05.2017 22:58

                  А куда засунуть switch, который автор заменяет фабрикой?


                  1. MrShoor
                    28.05.2017 23:03

                    В Action.Execute() же.


                    1. SirEdvin
                      28.05.2017 23:11

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


                      1. MrShoor
                        28.05.2017 23:37

                        Да этот Action.Execute() ровно так же распиливается на несколько приватных методов внутри.
                        Вы же считаете, что зачем-то обязательно надо создавать объекты с методом execute(). Ну а коль создавать объекты — то вот вам и фабрика. Не просто так же их создавать.


                        1. tmn4jq
                          28.05.2017 23:53

                          Я так понимаю, в этот самый execute вы хотите передавать object.getField(). В итоге execute будет содержать всю ту же логику, что и изначально была указана в посте. Да, разбиение на приватные методы – это неплохо, но, грубо говоря, в итоге получится все то же самое, только с еще одним уровнем вложенности.

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


              1. tmn4jq
                28.05.2017 23:06

                Про Java не ошибаетесь. Можно сделать статичный метод где-то в Utils-классе – таким образом сможем отвязать конкретный метод от какого-либо класса. Но, если честно, я не понимаю вашего спора. Ваш оппонент, уважаемый MrShoor изначально немножко попутал в установках: фабрика применяется не к условиям, а к самой конструкции switch. Да – само собой, сам этот switch, да и проверка условий, никуда не денутся – они просто уйдут в фабрику и в реаизацию, но сам высокоуровневый код станет легче.
                Пример из поста не вымышленный – я взял его из одного из своих проектов, просто существенно упростил код. Изначально это был ад – приходилось менять метод по каждому чиху. В целом я тоже не ярый сторонник всех принципов, сформулированных Бобом Мартином, и не любитель подхода «делай больше классов», но SRP – это все же важная штука. Лучше поменять пару строк в конкретной реализации (в одной из них), чем лезть в более общий класс ради каждого мелкого изменения.

                Но опять же, это просто более-менее абстрактный пример. Суть поста не в нем, и «ненависть» не зря написана в кавычках. В конце же указаны те преимущества, которые вы получите, прислушавшись к автору модуля.


    1. tmn4jq
      28.05.2017 22:50

      Так фабрика относится не к conditions, а к switch


      1. MrShoor
        28.05.2017 22:58
        +1

        Это значит, что когда мы хотим «обобщить» switch — для этого обязательно надо: создать фабрику, которой создать объект, чтобы вызвать execute? Почему нельзя просто сделать:

        execute(object.getField())
        
        без фабрики и лишних сущностей?


        1. tmn4jq
          28.05.2017 23:10
          -5

          Ну, как говорится, от перестановки слагаемых сумма не меняется. Представьте, что будет твориться в этом методе. Вы просто хотите перенести весь говнокод в отдельный метод, а не упростить код. А фабрика – удачное решение в данном моменте. Обратите внимание – она даже не абстрактная, а к тому же фабрика – привычный и удобный, и, главное, понятный для большинства программистов паттерн.


          1. MrShoor
            28.05.2017 23:58
            +6

            перенести весь говнокод в отдельный метод
            Я не вижу, почему вы этот код классифицировали как говнокод.
            Код понятен.
            Код легко можно отрефакторить. Обычно это делает тот человек, который хочет реюзнуть кусок.
            Код не использует не очевидные возможности и грязные хаки.
            По какому из ваших критериев он попал в говнокод?
            Для нового кода (имеется ввиду такого, которого еще нет в проекте. Т.е. я считаю что это не копипаста из другой функции) он в отличном состоянии.


            1. tmn4jq
              29.05.2017 00:09
              -6

              1. Код понятен – ну да, говнокод тоже зачастую понятен. Этот критерий не может использоваться ни мной, ни вами :)
              2. Вот именно. Хороший код не нужно рефакторить. Если этот код может быть зареюзан – это уже тема отдельного метода как минимум.
              3. Говнокод не в грязных хаках как таковых.

              Мне сложно донести всю ту суть, которую я хотел донести изначально, увы. Я же взял этот код из конкретного места в и курсе контекста, в отличие от читателей поста. Добавлю небольшую ремарку – этот код используется в эндпоинте. А эндпоинт, или контроллер – называйте как вам удобнее – это ближайшая к клиенту точка входа. И когда в этой ближайшей точке используется switch с кучей параметров – это не хорошо. Да-да, код совершенно понятен и последователен, но он всегда имеет больше, чем одну причину для изменения (Принцип SRP). Я не сомневаюсь, что вы знакомы с принципами SOLID, и данный код им ну никак не соответстует.

              Опять-таки, сложно судить, кто прав, исходя из абстрактного примера. Я не на этом делал акцент, в конце концов.


              1. MrShoor
                29.05.2017 00:37
                +7

                1. Код понятен – ну да, говнокод тоже зачастую понятен. Этот критерий не может использоваться ни мной, ни вами :)
                Зато обратный критерий можно использовать. Код непонятен — говнокод (исключение составляют сложные алгоритмы, к которым обычно пейпер сначала идет с математическими доказательствами). Ваш пример с фабриками менее понятен, чем тот, который на switch|if-ах. Если бы я был ревьюером этого кода — я бы сказал выпилить эти фабрики нафиг, а сделать по тупому, по влобному через switch (полагаем, что это новый код, и его еще никто не реюзал).

                2. Вот именно. Хороший код не нужно рефакторить.
                Это утопия. В долгоподдерживаемых продуктах постоянно что-то меняется. Идеальный код и архитектуру построить невозможно. Можно конечно учесть что-то на будущее, но тут явно не тот случай. А если код новый (как в примере) — то вообще неизвестно как он приживется. Может через месяц-два там что-то кардинально поменяется, и надо будет расширять логику.

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

                Да, наверное соглашусь. Предлагаю на этом закончить.


                1. vlad_iv
                  30.05.2017 16:49
                  -1

                  Александр, данный пример относится к Java и ООП.
                  Как Артем пытался донести, конструкция switch — это антипаттерн, характерный для процедурного, плохо поддерживаемого кода.
                  За подробной аргументацией, могу рекомендовать книгу: «Clean Code», Robert Martin
                  Причина: нарушение принципа открытости/закрытости: https://en.wikipedia.org/wiki/Open/closed_principle


                  1. MacIn
                    30.05.2017 20:57
                    -1

                    Как Артем пытался донести, конструкция switch — это антипаттерн, характерный для процедурного, плохо поддерживаемого кода.

                    С таким же успехом можно пытаться донести, что Земля — плоская.


  1. DoctorX
    28.05.2017 22:46
    +4

    Я так понял что статья сводится к тому что если вы ненавидите ревьюера то вы хреновый программист (или не паритесь). Вы пишите всё неправильно и злитесь когда вам на это указывают, а ревьюер априори прав. И это доказывается через два вымышленных примера.
    Пост больше похож на email конкретному сотруднику чем на статью из которой можно извлечь пользу широкому кругу программистов.


    1. tmn4jq
      28.05.2017 23:17

      Ну, на самом деле я хотел донести немного другую мысль: если вы гордитесь своим кодом, но получаете кучу замечаний насчет него – не торопитесь возмущаться и постарайтесь прислушаться. Вам же не зря эти замечания написали. Осознав их в полной мере, вы сможете не только написать более правильный код, но и лучше познакомиться с системой.
      P.S.: примеры из практики.


      1. Djeux
        29.05.2017 10:23
        -1

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


        1. Source
          29.05.2017 11:52
          +2

          change-request из-за отсутствия пустой строки в конце файла

          Это либо принято за правило в конкретном проекте, либо — нет. Всё-таки лучше соблюдать единообразие.


          недостаток комментариев в коде по мнению код-ревьюера

          Как правило, это мягкая формулировка "WTF?" по отношению к некоторым участкам кода. Т.е. если это не абстрактное "надо бы побольше комментариев", а с указанием неочевидных мест в коде, то это вполне объективный косяк.


          1. Djeux
            29.05.2017 11:57

            Речь именно об абстрактном.


      1. ad1Dima
        29.05.2017 10:29

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


        1. tmn4jq
          29.05.2017 10:38

          Ну от случая к случаю конечно, всегда надо и своей головой подумать


  1. mmxdesign
    28.05.2017 22:49
    -1

    По теме могу сказать что программисты часто свою работу принимают очень близко к сердцу, и любую критику или комментарий в адрес кода воспринимают лично на себя…
    Особенно это происходит когда код ревью делает по рангу и опыту равный, например Senior -> Senior
    Тут могут быть вовлечены конкуренция, личное эго или неприязнь.
    Конечно личные отношения между ревьювером также играет роль.



  1. superyateam
    29.05.2017 08:11
    -5

    автор, скажите «спасибо», что у вас на проекте такие ревьюеры, которые дотошно все проверяют и отлавливают ошибки до того, как они попадают в код! Лучше чем другая крайность, где нет ревью или ставят апрувы автоматом.

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


    1. tmn4jq
      29.05.2017 11:09

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

      А по поводу остальной части вашего комментария… ничего личного, но вот про таких «хреновых программистов» я и писал – отсюда и код «на отвали» (безотносительно споров switch-factory). Платят вам не за часы, а за работу, которую (и как) вы делаете.


    1. Daniyar94
      29.05.2017 21:47

      Вы серьезно?


  1. 7ft
    29.05.2017 10:23
    +1

    1. Согласен с теми «хреновыми программистами», которые за простой switch-case.

    2. У меня вопрос, зачем нужно делать по сути одно действие удаление (логическое или физическое) двумя разными способами?

    Tracker.untrack_object(o)
    Entity.delete(o)

    и
    Tracker.archive(o)
    o.archive()


    Возможно, лучше было бы прикрыть сверху единым методом delete(boolean archiving)?


    1. tmn4jq
      29.05.2017 10:39

      Можно, конечно, но вы снова увидели знакомые слова и конструкции и упустили саму мысль поста


      1. 7ft
        29.05.2017 10:54
        -1

        Я к тому, что код-ревьюер может быть не прав или прав не на 100%. Тут (как и везде в командной работе) важно взаимодействие во все стороны. Если мысль статьи в корне отличается от этой, будьте добры, расшифруйте.

        1. Код-ревьюер может прислушаться или просто задуматься, насколько необходимо городить фабрики.

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

        Т.е. и в первом и во втором пункте не вижу вины разработчика модуля номер два или его вины на 100%) В статье прекрасный пример случая, когда код-ревьюер должен в первую очередь подумать о себе и своей работе и только потом о другом программисте.


        1. tmn4jq
          29.05.2017 11:07
          -1

          Тут просто слишком много «если». Их все описывать – пост в кашу превращать.
          Я погнял, что вы имеете в виду, и все эти моменты имеют место. Но я хотел рассмотреть конкретный кейс, когда ревьюер действительно прав, и какие плюсы вы (и вся команда по сути) можете вынести из его комментариев.


    1. GerrAlt
      29.05.2017 10:45

      Это может быть не всегда возможно, в силу самых разных причин. Например в Tracker нельзя изменять состояние entity( в силу ограничений границ транзакции, например), а entity ничего не знает про tracker.


      1. 7ft
        29.05.2017 11:00

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


    1. Reposlav
      29.05.2017 18:43

      Чисто визуально лучше отличается delete() от archive()/softDelete(), чем delete(false) от delete(true), тем более, что еще нужно помнить или уточнять, какое значение, false или true, включает режим архивации.


      1. MacIn
        29.05.2017 19:25

        True и False могут быть заданы константами с говорящими именами.
        Например как True -> cArchive, использовать как
        delete(cArchive) / delete(not cArchive).


        1. Reposlav
          05.06.2017 12:56

          Есть еще принцип единственной ответственности. Один метод должен или удалять, или архивировать. Общий метод будет иметь более сложную логику.

          О существовании констант с говорящим именем еще нужно знать и помнить, IDE вам такое не подскажет. Можно, конечно, использовать ENUM'ы, если они есть в языке (в PHP их нет). Но не через чур ли это сложно в данном случае?


          1. MacIn
            06.06.2017 22:06

            Есть еще принцип единственной ответственности. Один метод должен или удалять, или архивировать. Общий метод будет иметь более сложную логику.

            Это рекомендация, на применимость которой надо смотреть в конкретном случае, а не догма.

            О существовании констант с говорящим именем еще нужно знать и помнить, IDE вам такое не подскажет. Можно, конечно, использовать ENUM'ы, если они есть в языке (в PHP их нет). Но не через чур ли это сложно в данном случае?

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


            1. ad1Dima
              07.06.2017 07:44

              в c# можно явно указать параметр по имени detele(toArchive:true);

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


      1. Azoh
        30.05.2017 00:59

        А вот тут, кстати, очень хорошо бы помогло бы ООП и патерны. Удаление бы происходило неким сервисом, который (в зависимости от настроек для каждой entity) уже бы знал, что нужно либо архивировать, либо удалять.


        1. Chamie
          30.05.2017 10:10

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


  1. 7ft
    29.05.2017 10:53
    -1

    (написал пост в другой ветке, этот можно удалить)


  1. AslanKurbanov
    29.05.2017 12:08
    +2

    Программирование становиться похожим на радиоэлектронику, которая проходила уже этот путь:

    Вот простая схема- несколько обычных переключателей-свичей, понятных сельскому электрику и обывателю. Починить легко, заменить не проблема. (Вспомним, к примеру, php в начале нулевых).

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

    По-моему, все идет по такому же пути, программное обеспечение будут делать на высокотехнологичных фабриках ПО и любительское программирование в основном уйдет в прошлое, как сгинули в небытие кружки радиолюбителей(в докомпьютерную эпоху — массовый тренд).


  1. NLO
    29.05.2017 15:24

    НЛО прилетело и опубликовало эту надпись здесь


    1. tmn4jq
      29.05.2017 15:28
      +1

      Там default, а не else. Но исключения там не будет


  1. Germes_Fox
    29.05.2017 15:57

    Ох, как знакомо мне это стало, когда меня перевели на проект, который сменил уже несколько программистов, а его архитектор был моим ревьювером :D