Должна ли быть включена эта проверка в Checkstyle? - PullRequest
Новая
       42

Должна ли быть включена эта проверка в Checkstyle?

6 голосов
/ 09 ноября 2009

Одна из встроенных проверок Checkstyle - RequireThis , которая сработает всякий раз, когда вы не добавляете this. к локальным вызовам полей или методов. Например, 

public final class ExampleClass {
    public String getMeSomething() { 
        return "Something"; 
    }

    public String getMeSomethingElse() {
        //will violate Checkstyle; should be this.getMeSomething()
        return getMeSomething() + " else"; 
    }
}

Я борюсь с тем, оправдана ли эта проверка. В приведенном выше примере ExampleClass является окончательным, что должно гарантировать , что должна быть вызвана «правильная» версия getMeSomething. Кроме того, могут быть случаи, когда подклассы могут переопределять поведение по умолчанию, и в этом случае требование «this» является неправильным поведением.

Наконец, кажется, что чрезмерно защитное поведение при кодировании только загромождает источник и затрудняет понимание того, что на самом деле происходит .

Итак, прежде чем я скажу своему архитектору, что это плохая проверка для включения, мне интересно, если кто-нибудь еще включил эту проверку ? Вы обнаружили критическую ошибку в результате отсутствия this?

Ответы [ 5 ]

4 голосов
/ 09 ноября 2009

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

void setSomething(String something) {
    something = something;
}

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

void setSomething(String something) {
    this.something = something;
}

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

Настройки checkstyle позволяют вам сохранить эту полезную проверку для полей, опуская при этом ненужную проверку для методов путем настройки правила следующим образом: 

   <module name="RequireThis">
       <property name="checkMethods" value="false"/>
   </module>

Когда речь идет о методах, это правило не имеет реального эффекта, потому что вызов this.getMeSomething() или просто getMeSomething() не влияет на разрешение методов Java. Вызов this.getSomethingStatic() по-прежнему работает, когда метод является статическим, это не ошибка, это всего лишь предупреждение в различных средах разработки и инструментах статического анализа.

4 голосов
/ 09 ноября 2009

Я бы определенно выключил его. Использование this.foo() не является идиоматической Java и поэтому должно использоваться только при необходимости, чтобы сигнализировать о том, что в коде происходит что-то особенное. Например, в сеттер: 

void setFoo(int foo) {this.foo = foo;}

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

Я искренне удивлен, увидев это, как правило, в библиотеке CheckStyle.

3 голосов
/ 09 ноября 2009

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

this. не является обязательным для компилятора, и когда в среде IDE выполняется интеллектуальное форматирование, это также не требуется для пользователя. А написание ненужного кода является просто источником ошибок в этом коде (в этом примере: использование this. в некоторых местах и ​​не использование его в других местах).

3 голосов
/ 09 ноября 2009

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

Если честно, это не похоже на частую проблему, я лично не думаю, что это стоило компромисса.

1 голос
/ 09 ноября 2009

Я бы включил эту проверку только для полей, потому что мне нравится дополнительная информация, добавляемая 'this.' перед полем.
См. Мой (старый) вопрос: Вы префикс вашей переменной экземпляра с "this" в Java? .

Но для любого другого проекта, особенно устаревшего, я бы не активировал его:

  • Скорее всего, ключевое слово 'this.' почти никогда не используется , что означает, что эта проверка будет генерировать тонны предупреждений.
  • переопределения имен (например, поле и метод с похожим именем) очень редки из-за того, что текущая IDE помечает по умолчанию этот код с собственным предупреждением.
...