Рефакторинг классов, которые используют глобальные переменные

Question

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

class MyClass {
    public void MyClass(Hashtable<String, String> params) {
        this.foo = GlobalClass.GLOBALVAR.get("foo");
        this.bar = GlobalClass.GLOBALVAR.get("bar");
        this.params = params;
    }
}

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

Мое текущее решение - создать дополнительные методы конструктора и установщика по умолчанию для params, foo и bar.

class MyClass {
     // Other code still here for backwards compatibility.
     public void MyClass() {
         // Do nothing much.
     }
     public void setParams(Hashtable<String, String> params) {
         this.params = params;
     }
     public void setFoo(Foo foo) {
         this.foo = foo;
     }
     public void setBar(Bar bar) {
         this.bar = bar;
     }
}

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

Answer 1

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

import java.util.Hashtable;


class MyClass
{
    private final Foo foo;
    private final Bar bar;
    private final Hashtable<String, String> params;

    public MyClass(final Hashtable<String, String> params)
    {
        this(params, GlobalClass.GLOBALVAR);
    }

    // added constructor
    public MyClass(final Hashtable<String, String> params, 
                   final FooBar fooBar)
    {
        this.foo    = fooBar.getFoo();
        this.bar    = fooBar.getBar();
        this.params = params;
    }
}

class MySubClass
    extends MyClass
{
    public MySubClass(final Hashtable<String, String> params)
    {
        super(params);
    }

    // added constructor
    public MySubClass(final Hashtable<String, String> params, 
                      final FooBar fooBar)
    {
        super(params, fooBar);
    }
}

// unchanged
class GlobalClass
{
    public static Car GLOBALVAR;
}

// added interface
interface FooBar
{
    Foo getFoo();
    Bar getBar();
}

class Car
    // added implements
    implements FooBar
{
    private Foo foo = new Foo();
    private Bar bar = new Bar();

    public Object get(final String name)
    {
        if(name.equals("foo"))
        {
            return (foo);
        }

        if(name.equals("bar"))
        {
            return (bar);
        }

        throw new Error();
    }

    // added method
    public Foo getFoo()
    {
        return ((Foo)get("foo"));
    }

    // added method
    public Bar getBar()
    {
        return ((Bar)get("bar"));
    }
}

// unchanged
class Foo
{
}

// unchanged
class Bar
{
}

Answer 2

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

interface GlobalVars {
   String get(String key);
}

Вы должны представить конструктор с ограниченной областью действия, вероятно, package-private

MyClass(GlobalVars globals, Map<String, String> params) {
   // create the object
}

А затем предоставьте public static фабричные методы для использования этого конструктора.

public static MyClass newMyClass(Map<String, String> params) {
   return new MyClass(GlobalClass.GLOBAL_VAR, params);
}

С этим проектом вы можете передать фиктивную реализацию GlobalVars в модульном тесте из того же пакета, явно вызвав конструктор.

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

private final Map<String, String> params;

Кроме того, сделайте защитную копию для предотвращения l33t h4x.

this.params = Collections.unmodifiableMap(params);

Answer 3

Ваш класс должен принимать все свои зависимости в конструкторе. Хорошая идея - сделать невозможным создание недопустимого или неинициализированного экземпляра классов. Сделайте foo и bar частными и окончательными и установите их в конструкторе.

Answer 4

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

Answer 5

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

• Измените базовый ctor MyClass, чтобы он принимал 3 параметра params, foo и bar. Закомментируйте ссылки GlobalVar и просто кешируйте передаваемые значения
• Компиляция .. это должно вызвать кучу ошибок компиляции - нет ctor, который принимает 1 параметр.
• Исправьте каждый из них для передачи в GlobalVar.get ("foo") и GlobalVar.get ("bar"). Получите это, чтобы построить.
• Уточнение: теперь минимизируйте попадания в БД путем ленивой загрузки и кэширования значений foo и bar. Выставлять через какое-то свойство на GlobalVar.

Answer 6

Этот GlobalClass.GLOBALVAR должен быть разбит на логические единицы. Таким образом, было бы проще создавать фиктивные объекты для модульных тестов. Например, в моей программе резки металла в CAD / CAM у меня есть MaterialList, SheetSizeList, PartNestingParameters и т. Д.

У меня нет огромного списка переменных в одном гигантском классе AppParameter. Все они свисают с объекта ShopStandards. Для модульного теста, включающего определенные PartNestingParmeters, я просто пойду ShopStandards.PartNestingParmeters = new MockPartNestingParameterTest114 (). Тест будет выполняться, не осознавая, что параметры вложения деталей являются макетом. Кроме того, это избавляет меня от необходимости выполнять десятки заданий только для того, чтобы правильно настроить ShopStandard для теста.

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