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

Question

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

public Triangle(List<Vertex> vertices) {
    this._values = vertices;
}

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

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

Я имею в виду, я, очевидно, могу сделать что-то подобное:

var triangle = new Triangle(new List<Vertex>(vertices));

но разве это не должно быть сделано создателями класса Triangle уже?

Я хотел бы знать некоторые рекомендации по этому вопросу. Спасибо.

Answer 1

Да, получающий класс (Треугольник) должен сделать копию, если проект не предназначен для намеренного совместного использования Списка.

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

Обратите внимание, что он все еще может делиться вершинами (элементами).

Answer 2

C # является языком передачи по значению, но поскольку список является ссылочным типом, он передает свою ссылку по значению. Как вы сказали, это означает, что вы передаете общую ссылку списка конструктору вашего класса. Изменения, сделанные в любом месте кода, будут влиять на тот же список.

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

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

Answer 3

Лично я согласен с Хенком; Вы должны создать копию.

    /// <summary>
    /// Initialises a new instance of the <see cref="Triangle"/> class that 
    /// contains elements copied from the specified collection.
    /// </summary>
    /// <param name="vertices">
    /// The collection of vertices whose elements are to be copied.
    /// </param>
    public Triangle(IEnumerable<Vertex> vertices)
    {
        this.Vertices = new List<Vertex>(vertices);
    }

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

Поэтому потребители знают, что могут смело звонить new Triangle(vertices).

Answer 4

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

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

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

2. Защитите себя от поведения класса, каким бы оно ни было.Если класс принимает изменяемый объект, не используйте этот объект повторно.Таким образом, даже если поведение класса изменится позже, вы в безопасности.

В вашем конкретном случае я не думаю, что класс Triangle неправильный.Его конструктор мог бы взять IEnumerable<Vertex> ¹ и инициализировать элемент List<Vertex> с этими значениями, но вместо этого его дизайнер решил взять List<Vertex> напрямую и использовать его.Это могло бы быть решение, основанное на производительности.

_{¹ В завершение, если немного педантично, я должен упомянуть, что даже если это заняло IEnumerable<Vertex>, вы моглидо сих пор сталкиваюсь с этой же проблемой.Класс все еще может хранить и повторно использовать ссылку на этот объект, и поэтому может быть чувствителен к изменениям, внесенным позже в список.В этом случае, однако, я будет считать класс Triangle нарушенным.Соглашение гласит, за небольшим исключением, что метод или конструктор, который принимает IEnumerable, будет использовать его один раз, а затем отбросит его.}

Answer 5

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

class Triangle
{
    List<Vertex> Vertices = new List<Vertex>(); // The triangle owns the vertex collection...

    public void SetVertices(IEnumerable<Vertex> vertices)
    {
        this.Vertices.Clear();

        this.Vertices.AddRange(vertices);
    }
}

Answer 6

То, что вам нужно, это клон или глубокая копия списка.

См. Этот ответ для клонирования списка

И это для более подробной информации о глубоких копиях, в общем