все совпадения абсолютно случайны.
Занимался я как-то раз улучшением кода проекта. И наткнулся вот на такие строчки:
public class Image
{
public Rectangle Bounds;
}
«Ай-яй-яй! Публичное поле в классе, как же нехорошо-то! Нужно срочно превратить его в свойство!» — подумал я. И превратил:
public class Image
{
public Rectangle Bounds { get; set; }
}
Сделал я такое невинное изменение и сразу пошёл дальше рефакторить — ведь ещё такое количество кода нуждалось в улучшении! Ну а в конце решил я запустить на всякий случай Unit-тесты. Какого же было моё удивление, когда половина тестов упала. «Да как же так! Ведь я особо-то ничего и не менял!» Ну, поехали разбираться.
Прежде всего взглянем на пресловутый Rectangle
:
public struct Rectangle
{
public int X, Y, Width, Height;
public void Inflate(int value) // «Раздуваем» наш прямоугольник
{
X -= value;
Y -= value;
Width += 2 * value;
Height += 2 * value;
}
public override string ToString()
{
return string.Format("[{0}, {1}, {2}, {3}]", X, Y, Width, Height);
}
}
Ну, а теперь сценарий использования:
var image = new Image();
image.Bounds = new Rectangle { X = 0, Y = 0, Width = 10, Height = 10 };
image.Bounds.Inflate(5);
Console.WriteLine(image.Bounds);
«Ах, ну зачем же так писать-то» — подумал я, ведь внезапно мне всё стало понятно. Давайте разбираться подробнее.
Случай до рефакторинга: Bounds — поле. В этом случае метод Inflate
будет работать с прямоугольником Bounds
, непосредственно относящимся к нашей картинке. Он успешно отработает, а метод Console.WriteLine
покажет нам «раздутую» версию границ: "-5 -5 20 20"
.
Случай после рефакторинга: Bounds — свойство. В этом случае настоящие границы хранятся в приватном сгенерированном поле, а при обращении к свойству
Bounds
на самом деле вызывается метод get_Bounds()
, который вернёт нам только копию прямоугольника. И раздувать мы будем уже копию, а не оригинал. Поэтому
Console.WriteLine
вернёт нам исходный прямоугольник: "0 0 10 10"
. Если уж нам так уж хочется сделать побольше оригинальный прямоугольник, то правильным способом будет являться следующий путь: достаём прямоугольник в локальную переменную, выполняем над ней необходимые манипуляции, а результат записываем обратно:
var bounds = image.Bounds;
bounds.Inflate(5);
image.Bounds = bounds;
Но если говорить более глобально, то архитектурная ошибка появилась в классе Rectangle
: нужно стараться не допускать в проекте изменяемых структур, их наличие может повлечь вышеописанные проблемы. «Но как же быть с публичными данными структуры, ведь до них я могу добраться явно и поменять их!» — спросите вы. На этот случай беспокоиться не стоит, компилятор C# — умный, он не будет даже компилировать строки вида
image.Bounds.X += 2;
Если вы хотите сделать методы, которые выполняют над структурой какие-то преобразования, то лучше бы этим методам создавать новый экземпляр структуры и возвращать его. Перепишем метод раздутия прямоугольника «правильным способом»:
public Rectangle Inflate(int value)
{
return new Rectangle
{
X = X - value,
Y = Y - value,
Width = Width + 2 * value,
Height = Height + 2 * value
};
}
Вот с таким методом заиметь проблемы по неосмотрительности уже намного сложнее. Мораль: если вы по каким-то причинам решили использовать в своём проекте структуры, то лучше бы вам не писать методов, которые изменяют их состояние.
Ссылки
- StackOverflow — Why are mutable structs evil?
- [http://habrahabr.ru/post/124404/"](Хабр — О вреде изменяемых значимых типов)
- О вреде изменяемых значимых типов. Часть 2