Объектно-ориентированный или последовательный? - PullRequest
6 голосов
/ 25 ноября 2008

Я выполняю рефакторинг 500 строк кода C ++ в main () для решения дифференциального уравнения. Я хотел бы заключить большие идеи нашего решателя в меньшие функции (то есть «SolvePotential (...)» вместо 50 строк числового кода).

Должен ли я кодировать это последовательно с помощью набора функций, принимающих очень длинные списки параметров, такие как:

int main(int *argc, void **argv){
   interpolate(x,y,z, x_interp, y_interp, z_interp, potential, &newPotential);
   compute_flux(x,y,z, &flux)
   compute_energy(x,y,z, &eng)
   ...
   // 10 other high-level function calls with long parameter lists
   ...
   return 0;
}    

Или я должен создать класс "SolvePotential", который будет называться так:

int main(int *argc, void **argv){
   potential = SolvePotential(nx, ny, nz, nOrder);
   potential.solve();
   return 0;
}

Где бы я определял функции в SolvePotential, которые используют переменные-члены, а не длинные списки параметров, такие как:

SolverPotential::solve(){
  SolvePotential::interpolate()
  SolverPotential::compute_flux()
  SolverPotential::compute_energy()
  // ... 
  //  10 other high-level function calls with NO parameter lists (just use private member variables)
}

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

Может быть, это все равно что спорить «12» или «одна дюжина»? », Но как вы думаете?

Ответы [ 12 ]

5 голосов
/ 25 ноября 2008

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

Кроме того, класс SolvePotential не имеет большого смысла, поскольку класс должен быть объектом с методом SolvePotential.

3 голосов
/ 26 ноября 2008

Ни. «Переместить весь мой код из одной функции в один класс» - это не ООП. Одним из фундаментальных правил ООП является то, что класс должен иметь одну зону ответственности . Это не единственная ответственность, это около 15:

SolverPotential::solve(){
SolvePotential::interpolate()
SolverPotential::compute_flux()
SolverPotential::compute_energy()
// ... 
//  10 other high-level function calls with NO parameter lists (just use private member variables)
}

Это также делает практически невозможным сохранение инварианта класса, не так ли? Когда можно вызывать compute_flux? Решать? Интерполяция? Что мешает мне сделать это в неправильном порядке? Будет ли класс в действительном состоянии, если я сделаю это? Получу ли я из него действительные данные?

Однако, почему это или-или? Почему нельзя сделать несколько классов и функций?

// This struct could be replaced with something like typedef boost::tuple<double,double,double> coord3d
struct coord3d {
double x, y, z;
};

coord3d interpolate(const coord3d& coord, const coord3d& interpolated, double potential); // Just return the potential, rather than using messy output parameters
double compute_flux(const coord3d coord&flux); // Return the flux instead of output params
double compute_energy(const coord3d& coord); // And return the energy directly as well

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

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

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

Если вам нужно составить ряд функций, то, скорее всего, вам нужно склеить их, чтобы сформировать новые, более широкие функциональные возможности, функциональное программирование и функторы. Одна из общих причин (но определенно не единственная) желательности составных функций заключается в том, что вам нужно выполнять одну и ту же операцию над множеством различных наборов данных (возможно, даже с несколькими разными типами, каждый из которых реализует одну и ту же концепцию). Заставив функтор выполнять тяжелую работу, можно использовать его с std :: transform или std :: for_each. Вы также можете использовать каррирование для постепенной сборки ваших функций (возможно, некоторые функции могут быть параметризованы набором фиксированных параметров, которые не меняются в зависимости от вызовов). Опять же, создайте функтор, который инициализируется с этими фиксированными параметрами, а затем предоставьте переменные данные в operator ().

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

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

Не зацикливайтесь на ООП. Используйте инструменты в вашем распоряжении.

Я не знаю достаточно контекста вашего вопроса, чтобы сказать наверняка, но мне кажется, что вам действительно нужен не класс, это просто иерархия функций. Ваш код пользователя звонит решить (). Внутренние вызовы solve (), скажем, (для примера, составлены), interpolate () и compute_energy (). compute_energy () внутренне вызывает compute_flux () и так далее. Каждая функция выполняет только пару вызовов для выполнения логических шагов, которые составляют ответственность функции. Так что нигде у вас нет огромного класса с дюжиной различных обязанностей или большой монолитной функцией, которая делает все последовательно.

В любом случае, в «очень длинных списках параметров» нет ничего плохого (обычно их можно сократить, сгруппировав некоторые из них вместе, но даже если вы не можете, нет ничего «un-OOP» в передаче много параметров. Напротив, это означает, что функция хорошо инкапсулирована из всего остального. Все, что ей нужно, передается в параметрах, поэтому она не привязана к остальной части приложения.

3 голосов
/ 25 ноября 2008

«SolvePotential» - это глагол, а классы, как правило, являются существительными с прикрепленными глаголами. Я не знаю много о деталях вашей проблемы, но это может быть признаком того, что процедурный подход будет более понятным, чем ОО здесь. В любом случае, похоже, что если бы вы создали этот класс, это было бы немного больше, чем упаковка для функций.

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

2 голосов
/ 25 ноября 2008

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

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

Кстати, с точки зрения функций, использование возвращаемых значений вместо параметров ввода / вывода обычно делает API более понятным.

Я бы даже осмелился сказать, что вы можете смешивать ОО и процедуры, используя классы для концепций , таких как Векторы (я вижу некоторые x, y, z вокруг). Это также приведет к удалению некоторых параметров, если это то, что вас так беспокоит.

float SolvePotential(const Vector3& vn, float nOrder)
{
    // ...
    const float newPotential = interpolate(vn, v_interp, potential);
    const float flux         = compute_flux(vn);
    const float energy       = compute_energy(vn);
    // ...
    return result;
}

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

Надеюсь, это поможет!

1 голос
/ 25 ноября 2008

Я голосую за класс, поскольку он упаковывает данные в гораздо более аккуратный пакет и делает вашу функцию main () вполне понятной.

В некотором смысле вы очистили функцию main (), и теперь у вас есть грязный класс, который вы можете очистить по своему усмотрению. Вид метода «разделяй и властвуй». Или, может быть, метод «впихнуть все мое барахло на чердак», где, по крайней мере, самая используемая часть дома чистая.

0 голосов
/ 12 декабря 2008

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

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

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

0 голосов
/ 02 декабря 2008

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

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

Боюсь, что я не знаю синтаксиса C ++, поэтому вместо него я буду использовать Java.

public class ValuePlusOne implements Computable {
    private int value;
    private int result;
    private Boolean hasRun;
    private static Map instanceMap = new HashMap();

    // Creates an instance reusing an existing one if possible
    public static getInstance(int value) {
        ValuePlusOne instance = (ValuePlusOne)instanceMap.get(value);

        if (instance = null) {
            instance = new ValuePlusOne(value);
            instanceMap.put(value,instance);
        }
        return instance;
    }

    // Private constructor
    private ValuePlusOne(int value) {
        this.value = value;
        hasRun = false;
    }

    // Computes (if not already computed) and returns the answer
    public int compute() {
        if (!hasRun) {
            hasRun = true;
            result = value + 1;
        }

        return result;
    }
}

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

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

0 голосов
/ 26 ноября 2008

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

В вашем примере кода, я думаю, что ваше имя класса немного не так. Сначала я бы применил рефакторинг, который вы делаете ( Извлечение метода , в меньшие функции). После этого проанализируйте, какие фрагменты данных идут с какими фрагментами логики, и конвертируйте процедурный дизайн в объекты (у Фаулера нет ссылки на этот «большой рефакторинг»).

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

PotentialSolution solve()

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

0 голосов
/ 26 ноября 2008

Вы забыли упомянуть середину, которая заключается в том, чтобы начать писать объект, но передать параметры его методам. ПСЕВДОКОД-лы:

SolverPotential::solve(a, b, c, d){
  SolvePotential::interpolate(a, b);
  SolverPotential::compute_flux(b, c);
  SolverPotential::compute_energy(c, d)

Таким образом, вы начинаете рефакторинг, думая в (предположительно более простом) последовательном режиме «что мне нужно под рукой, чтобы решить этот шаг?» Также, возможно, вы увидите последовательности параметров и повторов, которые предлагают деление объектов («После этого шага я никогда больше не использую« a ». Может быть, первые два шага должны быть заключены в другом классе»)

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

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

0 голосов
/ 25 ноября 2008

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

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

Я нахожусь в таком же положении, долгое время работал в Java и сейчас работаю в C / C ++. Вы проверяли, был ли это основной метод с 500 строками по причине? Существуют накладные расходы на создание экземпляров объектов и проверку виртуальных таблиц и тому подобного. Является ли производительность проблемой здесь? Кажется, что это будет в математических вычислениях, как это. Если это так, то все ставки сделаны с подходом Мартина-Фаулера.

Удачи.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...