Что не так с переопределенными вызовами методов в конструкторах?

У меня есть класс страницы Wicket, который устанавливает заголовок страницы в зависимости от результата абстрактного метода.

public abstract class BasicPage extends WebPage {

    public BasicPage() {
        add(new Label("title", getTitle()));
    }

    protected abstract String getTitle();

}

NetBeans предупреждает меня сообщением "Переопределенный вызов метода в конструкторе", но что с ним не так? Единственная альтернатива, которую я могу себе представить - это передать результаты абстрактных методов суперструктору в подклассах. Но это может быть трудно читать со многими параметрами.

8 ответов

Решение

При вызове переопределенного метода из конструкторов

Проще говоря, это неправильно, потому что это излишне открывает возможности для многих ошибок. Когда @Override вызывается, состояние объекта может быть непоследовательным и / или неполным.

Цитата из Effective Java 2nd Edition, Item 17: Разработка и документирование для наследования, или же запретить это:

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

Вот пример для иллюстрации:

public class ConstructorCallsOverride {
    public static void main(String[] args) {

        abstract class Base {
            Base() {
                overrideMe();
            }
            abstract void overrideMe(); 
        }

        class Child extends Base {

            final int x;

            Child(int x) {
                this.x = x;
            }

            @Override
            void overrideMe() {
                System.out.println(x);
            }
        }
        new Child(42); // prints "0"
    }
}

Здесь, когда Base вызов конструктора overrideMe, Child не закончил инициализацию final int x и метод получает неправильное значение. Это почти наверняка приведет к ошибкам и ошибкам.

Смежные вопросы

Смотрите также


На строительство объекта со многими параметрами

Конструкторы с множеством параметров могут привести к плохой читаемости, и существуют лучшие альтернативы.

Вот цитата из Effective Java 2nd Edition, Item 2: Рассмотрим шаблон компоновщика, когда сталкиваемся со многими параметрами конструктора:

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

Шаблон телескопического конструктора, по сути, выглядит примерно так:

public class Telescope {
    final String name;
    final int levels;
    final boolean isAdjustable;

    public Telescope(String name) {
        this(name, 5);
    }
    public Telescope(String name, int levels) {
        this(name, levels, false);
    }
    public Telescope(String name, int levels, boolean isAdjustable) {       
        this.name = name;
        this.levels = levels;
        this.isAdjustable = isAdjustable;
    }
}

И теперь вы можете сделать любое из следующего:

new Telescope("X/1999");
new Telescope("X/1999", 13);
new Telescope("X/1999", 13, true);

Вы не можете, однако, в настоящее время установить только name а также isAdjustable и уходя levels по умолчанию. Вы можете обеспечить больше перегрузок конструктора, но очевидно, что число будет взорваться при увеличении количества параметров, и вы можете даже иметь несколько boolean а также int аргументы, которые действительно испортили бы вещи.

Как вы можете видеть, это не приятный шаблон для написания, а еще менее приятный в использовании (что означает "истинный" здесь? Что такое 13?).

Bloch рекомендует использовать шаблон компоновщика, который позволит вам написать что-то вроде этого:

Telescope telly = new Telescope.Builder("X/1999").setAdjustable(true).build();

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

Смотрите также

Смежные вопросы

Вот пример, который помогает понять это:

public class Main {
    static abstract class A {
        abstract void foo();
        A() {
            System.out.println("Constructing A");
            foo();
        }
    }

    static class C extends A {
        C() { 
            System.out.println("Constructing C");
        }
        void foo() { 
            System.out.println("Using C"); 
        }
    }

    public static void main(String[] args) {
        C c = new C(); 
    }
}

Если вы запустите этот код, вы получите следующий вывод:

Constructing A
Using C
Constructing C

Ты видишь? foo() использует C до запуска конструктора C Если foo() требует, чтобы C имел определенное состояние (т.е. конструктор завершил работу), тогда он столкнется с неопределенным состоянием в C, и все может сломаться. И так как вы не можете знать в А, что перезаписано foo() ожидает, что вы получите предупреждение.

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

В вашем примере, что происходит, если подкласс переопределяет getTitle() и возвращает ноль?

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

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

class A {

    protected int minWeeklySalary;
    protected int maxWeeklySalary;

    protected static final int MIN = 1000;
    protected static final int MAX = 2000;

    public A() {
        setSalaryRange();
    }

    protected void setSalaryRange() {
        throw new RuntimeException("not implemented");
    }

    public void pr() {
        System.out.println("minWeeklySalary: " + minWeeklySalary);
        System.out.println("maxWeeklySalary: " + maxWeeklySalary);
    }
}

class B extends A {

    private int factor = 1;

    public B(int _factor) {
        this.factor = _factor;
    }

    @Override
    protected void setSalaryRange() {
        this.minWeeklySalary = MIN * this.factor;
        this.maxWeeklySalary = MAX * this.factor;
    }
}

public static void main(String[] args) {
    B b = new B(2);
    b.pr();
}

Результат будет на самом деле:

minWeeklySalary: 0

maxWeeklySalary: 0

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

Я надеюсь, что это поможет кому-то.

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

Посмотрите этот образец ссылки http://www.javapractices.com/topic/TopicAction.do?Id=215

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

Делая их частными, устраняет все сомнения: «Ты не пройдешь» .

Однако что, если вы ДЕЙСТВИТЕЛЬНО хотите, чтобы все было открыто?

Как я пытался объяснить настоящая проблема заключается не только в модификаторе доступа . Если честно, здесь,private это явная остановка там, где protected обычно по-прежнему допускает (опасный) обходной путь.

Более общий совет:

  • не запускайте потоки из вашего конструктора
  • не читайте файлы из вашего конструктора
  • не вызывайте API или сервисы из вашего конструктора
  • не загружайте данные из базы данных из вашего конструктора
  • не анализируйте документы json или xml из вашего конструктора

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

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

Вместо этого вы должны предоставить общедоступный init(), start() или connect()метод. А ответственность оставим на потребителя.

Проще говоря, вы хотите отделить момент « подготовки » от « зажигания ».

  • если конструктор можно расширить, он не должен самовоспламеняться.
  • Если он самовоспламеняется, он рискует быть запущенным до того, как будет полностью построен.
  • В конце концов, когда-нибудь в конструктор подкласса можно будет добавить больше подготовки. И у вас нет никакого контроля над порядком выполнения конструктора суперкласса.

PS: рассмотрите возможность реализации интерфейса Closeable вместе с ним.

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

  1. Строительство - через конструктор
  2. Инициализация - через onInitilize (после создания, когда работают виртуальные методы!)

Была довольно активная дискуссия о том, было ли это необходимо или нет (это полностью необходимо ИМХО), так как эта ссылка демонстрирует http://apache-wicket.1842946.n4.nabble.com/VOTE-WICKET-3218-Component-onInitialize-is-broken-for-Pages-td3341090i20.html)

Хорошая новость заключается в том, что отличные разработчики в Wicket действительно ввели двухфазную инициализацию (чтобы сделать самый крутой фреймворк Java UI еще более удивительным!), Так что с Wicket вы можете выполнить всю вашу инициализацию после конструирования в методе onInitialize, который вызывается фреймворк автоматически переопределяет его - на этом этапе жизненного цикла вашего компонента его конструктор завершил свою работу, поэтому виртуальные методы работают, как и ожидалось.

Я думаю, для калитки лучше позвонить add метод в onInitialize() (см. жизненный цикл компонентов):

public abstract class BasicPage extends WebPage {

    public BasicPage() {
    }

    @Override
    public void onInitialize() {
        add(new Label("title", getTitle()));
    }

    protected abstract String getTitle();
}
Другие вопросы по тегам