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

Question

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

Answer 1

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

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

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

Answer 2

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

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

Есть множество следствий:

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

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

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

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

Answer 3

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

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

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

Учтите это

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 4

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

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

Answer 5

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

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

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

Answer 6

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

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

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

Answer 7

Другой вариант, который вы могли бы рассмотреть, если вы используете один из языков dotnet, - пометить «старый» метод с помощью атрибута «Устаревший», который будет вводить все предупреждения компилятора, но при этом оставит код вызываемым, если он есть. код, который вы не можете контролировать (например, если вы пишете API или не используете строгий параметр в 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 8

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

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

http://www.refactoring.com/

Answer 9

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

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

Answer 10

Это «безопасно» в том смысле, что на языке, достаточно проверенном во время компиляции, оно заставляет вас обновлять все живые ссылки на то, что вы изменили.

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

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

void foo(int a, int b);

до

void foo(int a);

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

foo(1,2);

до:

foo(2);

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

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