четверг, 6 июня 2013 г.

Использование изменяемых структур в свойствах классов

Блог переехал. Актуальная версия поста находится по адресу: http://aakinshin.net/ru/blog/dotnet/mutable-structures-as-property/.


Названия всех классов вымышленные,
все совпадения абсолютно случайны.

Занимался я как-то раз улучшением кода проекта. И наткнулся вот на такие строчки:

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
    };
}

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

Для дополнительного чтения:

2 комментария:

  1. Модификация возвращаемого значения никак не отразится на работе Unit-тестов, лучше объявлять, методы, возвращающие новые экземпляры, статическими:

    public static Rectangle Inflate(int value)
    {
    return new Rectangle
    {
    X = X - value,
    Y = Y - value,
    Width = Width + 2 * value,
    Height = Height + 2 * value
    };
    }

    ОтветитьУдалить
    Ответы
    1. >>Модификация возвращаемого значения никак не отразится на работе Unit-тестов
      Это зависит от того, как написаны Unit-тесты (а их могли писать другие люди)
      >>лучше объявлять, методы, возвращающие новые экземпляры, статическими
      Это к обсуждаемой проблеме не относится. В общем случае вы можете объявить как статический метод, который возвращает новый прямоугольник, так и статический метод, который модифицирует внутренности текущего прямоугольника. А именно об этом и идёт речь, статика тут не при чём.

      Удалить