Является ли присвоение значения свойству через метод доступа set через метод доступа get его контейнера - это плохо?

Question

Рассмотрим следующий код:

class Program
{
    static void Main(string[] args)
    {
        Department deathStar = new Department { Name = "Death Star" };

        Console.WriteLine("The manager of {0} is {1}.", deathStar.Name, deathStar.Manager.FullName);

        deathStar.Manager.FirstName = "Lord";

        Console.WriteLine("The manager of {0} is {1}.", deathStar.Name, deathStar.Manager.FullName);

        Console.ReadLine();
    }
}

public class Person
{
    public string FirstName { get; set; }
    public string LastName { get; set; }

    public string FullName
    {
        get
        {
            return this.FirstName + " " + this.LastName;
        }
    }
}

public class Department
{
    public string Name { get; set; }

    public Person Manager { get; private set; }

    public Department()
    {
        this.Manager = new Person { FirstName = "Darth", LastName = "Vader" };
    }
}

, который выдает следующий вывод:

The manager of Death Star is Darth Vader.
The manager of Death Star is Lord Vader.

Несмотря на то, что я не могу изменить Manager на другой или новый экземпляр Person (средство доступа к частному набору), я могу изменить его свойства (которые имеют средства доступа к общедоступным наборам).

Итак, плохо ли присваивать значение свойству через метод доступа set через метод доступа get его контейнера? Другими словами, это запах кода?

EDIT:

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

Answer 1

Не обязательно.

Например, посмотрите на коллекцию параметров объекта SqlCommand. Вы можете изменить элементы в коллекции, но не можете назначить новую коллекцию параметров объекту команды.

Возьмем, к примеру, если у вас был пользовательский интерфейс, поддерживающий объекты Person, и вам нужно было изменить имя пользователя, то совершенно правильно изменить имя лица, оставить поля фамилии и PersonId в покое, а затем обновить таблицу базы данных используя поле PersonId.

Все это звучит нормально для меня

Answer 2

Да, это запах кода.

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

public interface Vehicle {
    double FuelTankCapacityInGallons{get;}
    double GallonsOfGasoline{get;}
}

public interface Vehicle {
    double getPercentFuelRemaining();
}

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

Пример, который я использовал, взят из книги Чистый код - Руководство по мастерству гибкого программного обеспечения Роберта Мартина.

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

Answer 3

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

В любом случае, мои мысли:
Пахнет.
Строка кода, которая действительно беспокоит меня:

deathStar.Manager.FirstName = "Lord";

Может легко поменяться на что-то вроде

deathStar.Manager.Address.City.Name = "Paris"

Итак, в одной строке кода вы видите 4 используемых класса! Это делает очень сильное предположение, что связь всех 4 классов остается неизменной. Другими словами, классы очень тесно связаны.
Вы можете решить эту проблему, работая с интерфейсами, а не с конкретными классами, и можете принять рекомендацию «Говорите, не спрашивайте» ( Поиск Yahoo ; да, Yahoo).
Вот хороший PDF , в котором обсуждаются ООП и рефакторинг (в Java, но идеи понятны легко), см. Раздел «Транзитивная связь».

Answer 4

Это не проблема, если Person не является структурой: если это так, свойство Manager вернет копию Person, и вы будете устанавливать свойство FirstName для этой копии, а не для Manager отдела. Но в любом случае компилятор закричит, если вы попытаетесь это сделать ...

Answer 5

Это "плохо"? Не по сути. Это признак того, что вы должны пересмотреть свой дизайн классов? Да. Почему то, что создает Department, модифицирует Person объект? Если вы вместо этого реализуете метод ChangeManager на Department? Может быть, а может и нет. Но этот вопрос стоит задать.

Answer 6

Я думаю, что это одна из причин, по которой люди так много говорят на функциональных языках - никаких «побочных эффектов». Я не думаю, что в этом есть что-то технически неправильное, но это может очень усложнить обслуживание. Перефразируя Парк Юрского периода, «... только потому, что вы можете что-то сделать, не значит, что вы должны!»

Answer 7

Я бы сказал, что строка:

deathStar.Manager.FirstName = "Lord";

совершенно ясно в своих намерениях, поэтому меня это не беспокоит.

Что меня беспокоит, так это личный набор на Manager.

Это означает, что после создания отдела его менеджер не может быть изменен.

Как образец для реальности, это большой запах времени.