C # - Анализ кода 2227 Путаница

Question

У меня есть свойство класса, которое выглядит следующим образом:

public List<Recipe> RecipeList
{
    get { return this._recipeList; }

    set
    {
        this._recipeList = value;
        OnPropertyChanged("RecipeList");
    }
}

В другом методе у меня есть следующее, которое ссылается на свойство выше.

private void RecipeSearch()
{
            this.RecipeList = RecipeManagerService.SearchByUnit(SearchCriteria)
                               .Where(recipe => recipe.IsApproved == true && !recipe.IsHidden).ToList();
}

Code Analysis выдает предупреждение CA 2227: измените RecipeList, чтобы он был доступен только для чтения, удалив установщик. Кто-нибудь может сказать мне, почему?

Answer 1

Добавление открытого сеттера на объект List<T> опасно. Вы можете устранить это предупреждение, сделав свой установщик приватным:

public List<Recipe> RecipeList
{
    get { return this._recipeList; }

    private set
    {
        this._recipeList = value;
        OnPropertyChanged("RecipeList");
    }
}

Это все еще позволит вашему классу изменить этот метод, но без внешнего источника.

Answer 2

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

Это всего лишь предложение:)

В этом случае вы будете использовать:

RecipeList.Clear();
RecipeList.AddRange(RecipeManagerService
                              .SearchByUnit(SearchCriteria)
                              .Where(r => r.IsApproved && !r.IsHidden));

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

Это также будет означать, что любой может изменить содержимое списка рецептов ... Вы определенно этого хотите? Другая альтернатива - выставить свойство ReadOnlyCollection<T> или что-то в этом роде и вносить изменения только в ваш собственный класс. Хотя это действительно зависит от того, что вы пытаетесь сделать.

Answer 3

Описание MSDN довольно ясно:

Свойство коллекции, доступное для записи, позволяет пользователю заменить коллекцию совершенно другой коллекцией

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

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

Answer 4

Хотите, чтобы другой экземпляр связывался с RecipeList?Обычно я не позволяю ничему изменять экземпляры моей коллекции, кроме экземпляра, которому принадлежит коллекция.Вы могли бы сделать это private.

Answer 5

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

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

Answer 6

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