Плох ли рефакторинг из-за ошибок компиляции?

Question

Меня использовали для выполнения некоторых рефакторингов путем введения ошибок компиляции.Например, если я хочу удалить поле из своего класса и сделать его параметром для некоторых методов, я обычно сначала удаляю поле, что вызывает ошибку компиляции для класса.Затем я бы ввел параметр в свои методы, который прерывал бы работу вызывающих устройств.И так далее.Обычно это давало мне чувство безопасности.На самом деле я не читал никаких книг (пока) о рефакторинге, но раньше я думал, что это относительно безопасный способ сделать это.Но мне интересно, действительно ли это безопасно?Или это плохой способ делать что-то?

Answer 1

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

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

Существует множество следствий:

• Если значение метода, функции или процедуры меняется, а тип также не меняется, то измените имя.(Когда вы тщательно изучите и исправите все варианты использования, вы, вероятно, снова измените название.)

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

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

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

Answer 2

Я никогда не полагаюсь на простую компиляцию при рефакторинге, код может компилироваться, но могли быть допущены ошибки.

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

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

Answer 3

Я не вижу в этом никакой проблемы.Это безопасно, и до тех пор, пока вы не вносите изменения перед компиляцией, это не имеет долгосрочного эффекта.Кроме того, в Resharper и VS есть инструменты, которые немного облегчают вам этот процесс.

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

Answer 4

Когда вы будете готовы прочитать книги на эту тему, я рекомендую книгу Майкла Физера "Эффективная работа с устаревшим кодом". (Добавлено неавтором:также классическая книга Фаулера "Рефакторинг" ... и тот Рефакторинг веб-сайт может быть полезен.)

Он рассказывает об определении характеристик кода, с которым вы работаете, прежде чем вносить изменения, и о выполнении того, что он называет рефакторингом с нуля.Это рефекторинг для поиска характеристик кода, а затем выбрасывание результатов.

То, что вы делаете, - это используете компилятор в качестве автоматического тестирования.Он проверит, компилируется ли ваш код, но не изменилось ли поведение из-за вашего рефакторинга или были ли какие-либо побочные эффекты.

Подумайте об этом

class myClass {
     void megaMethod() 
     {
         int x,y,z;
         //lots of lines of code
         z = mysideEffect(x)+y;
         //lots more lines of code 
         a = b + c;
     }
}

вы могли бы провести рефакторинг дополнения

class myClass {
     void megaMethod() 
     {
         int a,b,c,x,y,z;
         //lots of lines of code
         z = addition(x,y);
         //lots more lines of code
         a = addition(b,c);  
     }

     int addition(int a, b)
     {
          return mysideaffect(a)+b;
     }
}

и это сработало бы, но второе дополнение было бы неправильным, поскольку оно вызывало метод.Потребуются дополнительные тесты, помимо простой компиляции.

Answer 5

Достаточно легко представить примеры, когда рефакторинг из-за ошибок компилятора завершается безрезультатно и приводит к непреднамеренным результатам.
Несколько случаев, которые приходят на ум:(Я предположу, что мы говорим о C ++)

• Изменение аргументов функции, в которой существуют другие перегрузки с параметрами по умолчанию.После рефакторинга наилучшее соответствие аргументов может оказаться не таким, как вы ожидаете.
• Классы, которые имеют операторы приведения или неявные конструкторы с одним аргументом.Изменение, добавление, удаление или модификация аргументов к любому из них может привести к изменению наилучших совпадений, которые вызываются, в зависимости от используемого созвездия.
• Изменение виртуальной функции без изменения базового класса (или изменения базового класса по-другому) приведет к тому, что вызовы будут перенаправлены на базовый класс.

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

Answer 6

Я просто хотел бы добавить ко всей этой мудрости, что есть еще один случай, когда это может быть небезопасно.Размышление.Это влияет на такие среды, как .NET и Java (и другие тоже, конечно).Ваш код будет скомпилирован, но все равно будут возникать ошибки во время выполнения, когда Reflection попытается получить доступ к несуществующим переменным.Например, это может быть довольно распространенным явлением, если вы используете ORM, такой как Hibernate, и забываете обновлять свои XML-файлы сопоставления.

Возможно, было бы немного безопаснее искать имя конкретной переменной / метода по всем файлам кода.Конечно, это может привести к большому количеству ложноположительных результатов, так что это не универсальное решение;и вы могли бы использовать конкатенацию строк и в своем отражении, что также сделало бы это бесполезным.Но это, по крайней мере, на маленький шаг ближе к безопасности.

Однако я не думаю, что существует 100% надежный метод, если не считать прохождения всего кода вручную.

Answer 7

Я думаю, это довольно распространенный способ, поскольку он находит все ссылки на эту конкретную вещь.Однако современные IDE, такие как Visual Studio, имеют функцию Поиска всех ссылок, которая делает это ненужным.

Однако у такого подхода есть некоторые недостатки.Для крупных проектов компиляция приложения может занять много времени.Кроме того, не делайте этого в течение длительного времени (я имею в виду, верните все в рабочее состояние как можно скорее) и не делайте этого более чем для одной вещи одновременно, так как вы можете забыть правильный способ исправления ошибок в первый раз.

Answer 8

Это способ, и не может быть определенного утверждения о том, безопасен он или нет, без знания того, как выглядит код, который вы рефакторингуете, и какой выбор вы делаете.

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

http://www.refactoring.com/

Answer 9

Другой вариант, который вы могли бы рассмотреть, если вы используете один из языков dotnet, - это пометить "старый" метод устаревшим атрибутом, который будет отображать все предупреждения компилятора, но все же оставит вызываемый код, если есть код, находящийся вне вашего контроля (если вы пишете API или если вы не используете опцию strict в VB.Net , например).Затем вы можете провести веселый рефакторинг, заставив устаревшую версию вызвать новую;в качестве примера:

    public string Username
    {
        get
        {
            return this.userField;
        }
        set
        {
            this.userField = value;
        }
    }

    public int Login()
    {
        /* do stuff */
    }

Становится:

    [ObsoleteAttribute()]
    public string Username
    {
        get
        {
            return this.userField;
        }
        set
        {
            this.userField = value;
        }
    }

    [ObsoleteAttribute("Replaced by Login(username, password)")]
    public int Login()
    {
        Login(Username, Pasword);
    }

    public int Login(string username, string password)
    {
        /* do stuff */
    }

Во всяком случае, я склонен это делать именно так...

Answer 10

Это распространенный подход, но результаты могут отличаться в зависимости от того, является ли ваш язык статическим или динамическим.

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

У меня складывается впечатление, что вы работаете со статическим языком, таким как C # или Java, поэтому продолжайте использовать этот подход, пока не столкнетесь с какой-то серьезной проблемой, которая говорит, что вам следует поступить иначе.

Answer 11

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

Answer 12

Это звучит похоже на абсолютно стандартный метод, используемый в разработке, основанной на тестировании:напишите тест, ссылающийся на несуществующий класс, чтобы первым шагом для прохождения теста было добавление класса, затем методов и так далее.Видишь Книга Бека для получения исчерпывающих примеров Java.

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

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

Кстати, Eclipse делает этот метод "fail, stub, write" абсурдно простым в Java:каждый несуществующий объект помечен для вас, и Ctrl-1 плюс пункт меню указывают Eclipse написать (компилируемую) заглушку для вас!Мне было бы интересно узнать, предоставляют ли другие языки и IDE аналогичную поддержку.

Answer 13

Это "безопасно" в том смысле, что на языке, достаточно проверенном во время компиляции, это заставляет вас обновлять все текущие ссылки на то, что вы изменили.

Это все еще может пойти не так, если у вас есть условно скомпилированный код, например, если вы использовали препроцессор C / C ++.Поэтому убедитесь, что вы перестроили систему во всех возможных конфигурациях и на всех платформах, если это применимо.

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

void foo(int a, int b);

Для

void foo(int a);

Затем измените вызов с:

foo(1,2);

Для:

foo(2);

Это отлично компилируется, но это неправильно.

Лично я использую сбои компиляции (и связывания) как способ поиска в коде живых ссылок на функцию, которую я изменяю.Но вы должны иметь в виду, что это всего лишь устройство для экономии труда.Это не гарантирует, что результирующий код верен.