Есть ли что-то изначально неправильное в длинных цепочках вызова объектов?

Самый большой флаг в мире.

Вы не можете легко проверить, возвращает ли какой-либо из этих вызовов нулевой объект, что делает отслеживание любых ошибок практически невозможным!

getClientFile () может вернуть значение null, а затем getClient () завершится ошибкой, и когда вы поймаете это, при условии, что вы пытаетесь поймать, вы не будете знать, какой из них потерпел неудачу.

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

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

да. Это не лучшая практика. Во-первых, если есть ошибка, ее труднее найти. Например, ваш обработчик исключений может отображать трассировку стека, которая показывает, что у вас есть исключение NullReferenceException в строке 121, но какой из этих методов возвращает значение null? Чтобы узнать это, вам придется покопаться в коде.

Это субъективный вопрос, но я не думаю, что в нем что-то не так. Например, если цепочка выходит за пределы читаемой области редактора, вам следует ввести несколько местных жителей. Например, в моем браузере я не вижу последних 3 вызовов, поэтому я понятия не имею, что вы делаете :).

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

В противном случае я не вижу вреда в том, что делаете вы. Это конечно лучше чем

ActionPlan AP = task.getActionPlan();
ClientFile CF = AP.getClientFile();
Client C = CF.getClient();
DocFolder DF = C.getDocumentsFolder();

Что ж, каждое косвенное обращение добавляет одну точку, где все может пойти не так.

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

Поэтому, если есть вероятность, что какой-либо из методов может вернуть значение null, я бы по крайней мере разделил цепочку в этих точках и сохранил промежуточные переменные и разбил ее на отдельные строки, чтобы отчет о сбое дал мне номер строки для просмотра.

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

А как насчет удобочитаемости? Было бы понятнее, если бы вы добавили именованные переменные? Всегда пишите код так, как будто вы хотите, чтобы он был прочитан другим программистом, и только случайно интерпретироваться компилятором.

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

ActionPlan taskPlan = task.GetActionPlan();
ClientFile clientFileOfTaskPlan = taskPlan.GetClientFile();
Client clientOfTaskPlan = clientFileOfTaskPlan.GetClient();
File clientFolder = clientOfTaskPlan.getDocumentsFolder();

Думаю, дело в личном мнении по этому поводу.

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

И я считаю, что вам не нужно вводить переменные в свой код. Таким образом, объекты знают все, что им нужно для перехода от метода к методу. (Здесь возникает вопрос, если вы немного не изощрялись, но кто я такой, чтобы сказать?)

Я бы ввел своего рода «удобные методы». Представьте, что у вас есть метод в своей «задаче» - объект чего-то вроде

    task.getClientFromActionPlan();

Тогда вы наверняка могли бы использовать

    task.getClientFromActionPlan().getDocumentsFolder();

Гораздо более читабельным и, если вы правильно используете эти «удобные методы» (например, для цепочек часто используемых объектов), гораздо меньше набирать ;-).

Эдит говорит: Эти удобные методы, которые я предлагаю, часто содержат проверку Nullpointer, когда я их пишу. Таким образом, вы даже можете использовать Nullpointers с хорошими сообщениями об ошибках (например, «ActionPlan был нулевым при попытке получить от него клиента»).

Связанное обсуждение:

Цепочка функций - сколько это слишком много?

Этот вопрос очень близок к https://stackoverflow.com/questions/154864/function-chaining-how-many-is-too-many#155407

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

Хотя я слышал, что поклонники Python считают создание цепочек большим развлечением. Это может быть просто слух ... :-)

Во-первых, такое наложение кода может раздражать анализ исключений NullPointerExceptions и проверку ссылок при запуске отладчика.

Кроме того, я думаю, все сводится к следующему: Требуется ли вызывающему абоненту все эти знания?

Возможно, его функциональные возможности можно было бы сделать более универсальными; Вместо этого файл может быть передан в качестве параметра. Или, возможно, ActionPlan даже не должен раскрывать, что его реализация основана на ClientFile?

В зависимости от вашей конечной цели вы, вероятно, захотите использовать принцип наименьшего знания, чтобы избежать сильной связи и затрат в конечном итоге. Как первый любит выражаться голова .. «Говорите только с друзьями».

Я согласен с плакатом, в котором упоминался Закон Деметры. Вы создаете ненужные зависимости от реализаций многих из этих классов и от структуры самой иерархии. Это очень затруднит тестирование вашего кода изолированно, так как вам нужно будет инициализировать дюжину других объектов только для того, чтобы получить рабочий экземпляр класса, который вы хотите протестировать.

Еще один важный побочный продукт цепочки - производительность.

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

Цепочка также затрудняет оценку производительности, вы не можете сказать, какой из этих методов может или не может делать что-то сложное.

флаг красный, и он говорит о двух вещах в смелый:

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

и одно в скобках:

• используйте свойство, например, task.ActionPlan вместо task.getActionPlan ()

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

File clientFolder = task.DocumentsFolder;

это как минимум скроет древовидную структуру от вызывающего кода. Внутренне свойства могут выглядеть так:

class Task {
    public File DocumentsFolder {
        get { return ActionPlan.DocumentsFolder; }
    }
    ...
}
class ActionPlan {
    public File DocumentsFolder {
        get { return ClientFile.DocumentsFolder: }
    }
    ...
}
class ClientFile {
    public File DocumentsFolder {
        get { return Client.DocumentsFolder; }
    }
    ...
}
class Client {
    public File DocumentsFolder {
        get { return ...; } //whatever it really is
    }
    ...
}

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

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

Как вовремя. Я собираюсь написать сообщение в своем блоге сегодня вечером об этом запахе, Цепочки сообщений, в отличие от его обратного, Средний человек.

В любом случае, более глубокий вопрос заключается в том, почему у вас есть методы "get" для того, что выглядит как объект домена. Если вы внимательно проследите за контурами проблемы, вы либо обнаружите, что нет смысла сообщать задаче, чтобы что-то получить, либо что то, что вы делаете, на самом деле является проблемой, не связанной с бизнес-логикой, такой как подготовка к отображению пользовательского интерфейса, настойчивость, реконструкция объекта и т. д.

В последнем случае методы "get" подходят, если они используется авторизованными классами. То, как вы применяете эту политику, зависит от платформы и процесса.

Так что в случае, когда методы «get» считаются приемлемыми, вам все равно придется столкнуться с проблемой. И, к сожалению, я думаю, это зависит от класса, который перемещается по цепочке. Если уместно, чтобы этот класс был связан со структурой (скажем, фабрикой), то пусть будет. В противном случае вам следует попробовать Скрыть делегата.

Обновлено: нажмите здесь, чтобы увидеть мой пост.

Геттеры и сеттеры - зло. Как правило, избегайте того, чтобы объект что-то с ним делал. Вместо этого делегируйте саму задачу.

Вместо

Object a = b.getA();
doSomething(a);

делать

b.doSomething();

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

Я бы тоже указал на Закон Деметры.

И добавить статью про Скажи, не спрашивай

Собираетесь ли вы когда-нибудь использовать каждую из этих функций независимо? Почему бы просто не создать в задаче метод GetDocumentsFolder (), который выполняет всю грязную работу по вызову всех этих методов за вас? Затем вы можете заставить его выполнять всю грязную работу по проверке на нуль всего, не ломая свой код в тех местах, где его не нужно обрабатывать.

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

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

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

enum FolderType { ActionPlan, ClientFile, Client, etc }

interface IFolder
{
    IFolder FindTypeViaParent( FolderType folderType )
}

и каждый класс, реализующий IFolder, вероятно, просто выполняет

IFolder FindTypeViaParent( FolderType folderType )
{
    if ( myFolderType == folderType )
        return this;

    if ( parent == null )
        return null;

    IFolder parentIFolder = (IFolder)parent;
    return parentIFolder.FindTypeViaParent(folderType)
}

Другой вариант - сделать интерфейс IFolder:

interface IFolder
{
    FolderType FolderType { get; }
    IFolder Parent { get; }
}

Это позволяет экстернализовать код обхода. Однако это отнимает контроль у классов (возможно, у них несколько родителей) и предоставляет реализацию. Хорошо и плохо.

[бред]

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

Опять же, действительно ли иерархия является частью идентичности объекта? Если это не так, возможно, вы могли бы экстернализовать иерархию как n-арное дерево.

В качестве примечания вы можете рассмотреть концепцию агрегирования DDD (Domain Driven Design) и определить, кто является основными игроками. За что отвечает конечный собственник объекта? например колеса авто. В дизайне, моделирующем автомобиль, колеса также являются объектами, но они принадлежат автомобилю.

Может быть, это сработает для вас, а может и нет. Как я уже сказал, это всего лишь выстрел в темноте.

С другой стороны, Закон Деметры не применим повсеместно, ни жесткое правило (arrr, это быть скорее ориентиром!).