Ref Abuse: Стоит очистить?

Question

Я унаследовал некоторый код, который использует ключевое слово ref широко и излишне. Первоначальный разработчик, очевидно, опасался, что объекты будут клонированы, как примитивные типы, если ref не используется, и не потрудился исследовать проблему, прежде чем писать 50k + строк кода.

Это, в сочетании с другими плохими методами кодирования, создало некоторые ситуации, которые являются абсурдно опасными на поверхности. Например:

---

Customer person = NextInLine(); 
//person is Alice
person.DataBackend.ChangeAddress(ref person, newAddress);
//person could now be Bob, Eve, or null

Не могли бы вы представить, как вы зашли в магазин, чтобы сменить адрес, и вышли как совершенно другой человек?

---

Страшно, но на практике использование ref в этом приложении кажется безвредным излишним. У меня возникли проблемы с обоснованием того, сколько времени потребуется, чтобы его почистить. Чтобы помочь продать идею, я задаю следующий вопрос:

Как еще может ненужное использование ref быть разрушительным?

Я особенно обеспокоен обслуживанием. Правдоподобные ответы с примерами предпочтительны.

Вы также можете утверждать, что в очистке нет необходимости.

Answer 1

Я бы сказал, что самая большая опасность, если по какой-то причине параметр был установлен на null внутри функции:

public void MakeNull(ref Customer person)
{
    // random code
    person = null;
    return;
}

Теперь вы не просто другой человек, вы полностью стерты из существования!

Пока тот, кто разрабатывает это приложение, понимает, что:

По умолчанию ссылки на объекты передаются по значению.

и

С ключевым словом ref ссылки на объекты передаются по ссылке.

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

Answer 2

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

public bool DoAction(ref Exception exception) {...}

Да, вам пришлось объявить и передать ссылку на исключение, чтобы вызвать метод, а затем проверить возвращаемое значение метода, чтобы увидеть, было ли исключение поймано и возвращено через исключение ref.

Answer 3

Можете ли вы понять, почему создатель кода решил, что ему нужно иметь параметр в качестве ссылки? Было ли это потому, что они обновили его, а затем удалили функциональность, или просто потому, что в то время они не понимали c #?

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

Answer 4

Мне пахнет как разработчик C ++, делающий необоснованные предположения.

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

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

Я предлагаю вам убирать по ходу дела - по одному методу за раз.

1. Найдите метод, который использует ref, если вы уверены, что он не требуется.
2. Изменить подпись метода и исправить вызовы.
3. Test.
4. Повтор.

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

Оптовые "апгрейды" часто сталкиваются с трудностями. Рефакторинг по ходу дела - доведение вещей до спецификаций при работе над ними - намного безопаснее.

Здесь есть прецедент в строительной отрасли. Например, к электропроводке в более старых (скажем, 19-го века) зданиях не нужно прикасаться, если нет проблем. Когда является проблемой, новая работа должна быть выполнена в соответствии с современными стандартами.

Answer 5

В C # довольно распространено изменять значения аргументов в методах, поскольку они обычно являются значениями, а не ссылками. Это относится как к ссылочным, так и к типам значений; установка ссылки на ноль, например, изменит исходную ссылку. Это может привести к очень странным и болезненным ошибкам, когда другие разработчики работают «как обычно». Создание рекурсивных методов с аргументами ref не требуется.

Помимо этого, у вас есть ограничения на то, что вы можете передать по реф. Например, вы не можете передавать константные значения, поля только для чтения, свойства и т. Д., Поэтому при вызове методов с аргументами ref требуется много вспомогательных переменных.

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

Answer 6

Как еще может ненужное использование ссылки быть разрушительным?

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

Но ради интереса, как насчёт другого аспекта - производительности?

void ChangeAddress(ref Customer person, Address address)
{
    person.Address = address;
}

Здесь person является ссылкой на ссылку, поэтому при каждом доступе к ней будет вводиться некоторая косвенность. Давайте посмотрим на некоторую сборку, это может привести к:

mov eax, [person]           ; load the reference to person.
mov [eax+Address], address  ; assign address to person.Address.

Теперь версия без ссылки:

void ChangeAddress(Customer person, Address address)
{
    person.Address = address;
}

Здесь нет косвенного обращения, поэтому мы можем избавиться от одного чтения:

mov [person+Address], address  ; assign address to person.Address.

На практике можно надеяться, что .NET кеширует [person] в версии ref, амортизируя стоимость косвенного обращения при множественном доступе. Вероятно, на самом деле это не будет 50% -ным падением количества команд за пределами тривиальных методов, подобных приведенным здесь.

Answer 7

Я бы попытался это исправить. Просто выполните замену строки решения на регулярное выражение и проверьте после этого модульные тесты. Я знаю, что это может нарушить код. Но как часто вы используете реф? Почти никогда, верно? И учитывая, что разработчик не знал, как это работает, я считаю, что вероятность того, что он будет использован где-то (так, как должно), еще меньше. А если код сломается - ну, откат ...