Pull to refresh

Объектно-ориентированный антипаттерн

Reading time3 min
Views15K
Original author: Arthur O’Dwyer

Для тех, кто читает не далее второго абзаца: я не говорю, что любое ООП - это плохо! ООП, особенно классическое полиморфное ООП, заслуженно имеет своё место в реальных проектах. Ниже речь поидёт про частный случай (анти)паттерна, который я периодически встречаю: использование классов там, где могли быть простые свободные функции.

Начало

Довольно часто у студентов, изучающих C++ в определённых учебных кругах, складывается мировоззрение о том, что всё должно быть объектами. Попросите их написать программу, которая считает некоторое значение - и они начнут с создания объекта ValueComputer и метода vc.computeResult().

Например: дана задача с помощью динамического программирования посчитать количество способов замостить костяшками домино прямоугольник w \times h. Студент пишет:

int main()
{
    DominoTilingCounter tc(4, 7); // in a 4x7 rectangle
    std::cout << tc.count() << '\n';
}

Обозначив задачу, студент далее приступает к написанию класса DominoTilingCounter. А если он смышлёный, то также добавит мемоизацию, чтобы метод count() не считал каждый раз всё заново:

class DominoTilingCounter {
    int height, width;
    bool done = false;
    int tilingCount;
    int computeCount(int h, int w, std::string_view prevRow, int rowIdx) {
        [...recursive solution omitted...]
    }
public:
    explicit DominoTilingCounter(int h, int w) : height(h), width(w) {
        if (h == 0 || w == 0 || (w*h) % 2 != 0) {
            tilingCount = 0;
            done = true;
        }
    }
    int count() {
        if (!done) {
            tilingCount = computeCount(height, width, "", 0);
            done = true;
        }
        return tilingCount;
    }
};

(Если вам от этого кода захотелось фыркнуть - хорошо. Так и задумано.)

Здесь можно было бы придраться к тому, что метод count() по смыслу должен быть константным, но его таковым сделать нельзя, потому что он меняет внутренние поля класса. Давайте это исправим.

Продолжение, или рассуждаем логически

Смотрите: ты конструируешь объект DominoTilingCounter tc, только чтобы вычислить tc.count(), правильно? И больше объект ни для чего не используется?

Ещё раз: ты конструируешь объект DominoTilingCounter tc, только чтобы вычислить tc.count().

Так перенеси вычисление в конструктор!

class DominoTilingCounter {
    int height, width;
    int tilingCount;
    int computeCount(int h, int w, std::string_view prevRow, int rowIdx) {
        [...recursive solution omitted...]
    }
public:
    explicit DominoTilingCounter(int h, int w) : height(h), width(w) {
        if (h == 0 || w == 0 || (w*h) % 2 != 0) {
            tilingCount = 0;
        } else {
            tilingCount = computeCount(height, width, "", 0);
        }
    }
    int count() const {
        return tilingCount;
    }
};

Вот что изменилось после рефакторинга:

  • метод count() стал константным,

  • объект больше не может находиться в "невычисленном" состоянии,

  • убрали поле done, которое использовалось, только чтобы отличать это "невычисленное" состояние (кстати, в C++17 можно было использовать std::optional для тех же целей, но нам теперь и не надо).

Более того, поля width и height тоже больше не нужны. Они использовались, чтобы передать данные из конструктора в метод count(), а теперь все вычисления происходят в конструкторе, ничего никуда передавать не нужно. Код значительно сократился.

Конец

Так получилось, что с самого начала метод computeCount() не обращался к полям height и width класса, а получал данные через параметры w и h. По счастливому случаю метод computeCount() не обращается вообще ни к каким полям класса, значит, его можно пометить как static. Теперь наш код выглядит как-то так:

class DominoTilingCounter {
    int tilingCount;
    static int computeCount(int h, int w, std::string_view prevRow, int rowIdx) {
        if (h == 0 || w == 0 || (w*h) % 2 != 0) {
            return 0;
        }
        [...recursive solution omitted...]
    }
public:
    explicit DominoTilingCounter(int h, int w) {
        tilingCount = computeCount(h, w, "", 0);
    }
    int count() const {
        return tilingCount;
    }
};

Заметим, что, по сути, теперь класс просто оборачивает int!

Избегайте классов, по своей сути равнозначных int.

(А что насчёт float или double? - прим. перев.)

Что нам нужно было написать, это простую свободную функцию countDominoTilings(h, w):

int countDominoTilingsImpl(int h, int w, std::string_view prevRow, int rowIdx) {
    if (h == 0 || w == 0 || (w*h) % 2 != 0) {
        return 0;
    }
    [...recursive solution omitted...]
}

int countDominoTilings(int h, int w) {
    return countDominoTilingsImpl(h, w, "", 0);
}

int main() {
    int tc = countDominoTilings(4, 7);  // in a 4x7 rectangle
    std::cout << tc << '\n';
}

Никаких классов. Больше не нужно беспокоиться о константности методов. Можно забыть про мемоизацию (теперь это забота вызывающего кода). Первоначальный вариант с классом был потоко-небезопасен, и об этом тоже можно больше не переживать. А ещё наш код стал на десяток строк короче.

На всякий случай, ещё раз: не все классы - это плохо! Но если вам нужно что-то посчитать, не пишите класс ВычисляторЗначения. Напишите функцию вычислить_значение().

Tags:
Hubs:
Total votes 29: ↑16 and ↓13+8
Comments53

Articles