Злоупотребление ссылкой: стоит ли очистить?

Я унаследовал некоторый код, который использует ключевое слово 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 может быть разрушительным?

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

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

Ответ 1

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

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

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

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

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

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

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

Ответ 2

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

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

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

Ответ 3

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

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

Ответ 4

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

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

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

Ответ 5

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

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

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

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

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

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

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

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

Ответ 6

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

Ответ 7

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

Другие ответы касаются семантических вопросов, которые, безусловно, являются самыми важными. Хороший код самодокументирован, и когда я даю параметр 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.

Теперь не ref ref:

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

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

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

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