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

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

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

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

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

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

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

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

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


.net reference parameter-passing c#
person Dyppl    schedule 18.02.2011    source источник
comment
@Timwi: хорошо, я исправил ошибку. Я действительно думаю, что это тот случай, когда фактический код не имеет значения, потому что я спрашиваю не о данном конкретном случае, а о том, как все должно быть в целом, и это просто пример   -  person Dyppl    schedule 18.02.2011

Ответы (6)


arrow_upward
2
arrow_downward

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

    /// <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).

person Siy Williams    schedule 18.02.2011

arrow_upward
4
arrow_downward

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

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

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

person Henk Holterman    schedule 18.02.2011

arrow_upward
2
arrow_downward

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

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

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

person Jordan Parmer    schedule 18.02.2011

arrow_upward
1
arrow_downward

В этом случае передача объекта 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);
    }
}
person MattDavey    schedule 18.02.2011
comment
Потому что это вызывает проблемы, подобные описанной OP. Класс Triangle должен стать владельцем коллекции, которую он использует. Кроме того, он не требует дополнительных функций списка. В этом случае минимальная необходимая функциональность заключается в том, что Triangle может перечислять коллекцию вершин, поэтому IEnumerable является более связным и не тесно связанным с какой-либо конкретной реализацией коллекции. - person MattDavey; 21.02.2011

arrow_upward
1
arrow_downward

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

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

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

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

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

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

person P Daddy    schedule 18.02.2011
comment
Это очень хороший способ принять безопасный курс действий, спасибо - person Dyppl; 18.02.2011

arrow_upward
0
arrow_downward

Вам нужен клон или глубокая копия списка.

Обратитесь к этому ответу для клонирования списка

И это чтобы узнать больше о глубоком копировании в целом

person Robin Maben    schedule 18.02.2011
comment
Клонирование списка не проблема, я просто хочу знать, кто должен это делать - person Dyppl; 18.02.2011