Есть ли способ заменить две подобные функции, которые отличаются только типом, одним решением в этом случае? - PullRequest
1 голос
/ 18 марта 2019

Есть класс с именем PlotCurve.Он описывает диаграмму как контейнер точек и операций над ними.Данные для PlotCurve получены из класса RVDataProvider.Важно то, что количество точек, предоставляемых RVDataProvider, может быть большим (более 1kk), поэтому RVDataProvider возвращает доступный только для чтения указатель на данные Y (данные X могут быть рассчитаны по индексу указателя) для улучшенияисполнение.

Основная проблема заключается в том, что RVDataProvider имеет два разных метода для двух типов:

class RVDataProvider : public QObject, public IRVImmutableProvider
{
public:
    // ...
    ReadonlyPointer<float>  getSignalDataFloat(int signalIndex, quint64 start, quint64 count) override;
 ReadonlyPointer<double> getSignalDataDouble(int signalIndex, quint64 start, quint64 count) override;
    // ...
}

ReadonlyPointer<T> - это только оболочка только для чтения указателя в стиле C.

Чтобы получить диапазон значений кривой (для поиска min-max, рисования их на холсте и т. Д.), Я также должен объявить различные функции.

class PlotCurve : public QObject
{
public:
    // ...` 
    virtual ReadonlyPointer<float> getFloatPointer(quint64 begin, quint64 length) const;
    virtual ReadonlyPointer<double> getDoublePointer(quint64 begin, quint64 length) const;  
    // ...
}

Это приводит киспользование оператора switch в клиентском коде и его изменения при добавлении нового доступного типа данных.

switch (dataType())
{
    case RVSignalInfo::DataType::Float: {
        auto pointer = getFloatPointer(begin, length);
        Q_ASSERT(!(pointer).isNull()); \
        for (quint64 i = 0; i < (length); ++i) { \
            auto y = (pointer)[i]; \
            if (y < (minY)) { (minY) = y; continue; } \
            if (y > (maxY)) { (maxY) = y; } \
        }
    } break;

    case RVSignalInfo::DataType::Double: {
        auto pointer = getDoublePointer(begin, length);
        Q_ASSERT(!(pointer).isNull()); \
        for (quint64 i = 0; i < (length); ++i) { \
            auto y = (pointer)[i]; \
            if (y < (minY)) { (minY) = y; continue; } \
            if (y > (maxY)) { (maxY) = y; } \
        }
    } break;

    // ...
}

Есть ли способ избавиться от зависимостей от клиентского кода?Мне в голову пришло три вещи:

1) Создать тип Iterator, который будет оберткой ReadonlyPointer.Нет - производительность снижается в 10 и более раз из-за виртуальных функций итератора.

2) Создайте метод обхода, который будет выполнять некоторую функцию для каждого значения в некотором диапазоне.Нет снова - самая оптимизированная версия, использующая указатели функций, в два раза медленнее, чем оператор switch в клиентском коде.

3) Создайте шаблон класса PlotCurve.Таким образом, я не могу добавить разные PlotCurves в один контейнер, как сейчас.

1 Ответ

1 голос
/ 18 марта 2019

К сожалению, я не вижу многого, что можно сделать для проблемы ОП.

В лучшем случае похожие части корпусов можно перенести в

  1. макрос
  2. шаблон функции

для предотвращения дублирования кода.

Для демонстрации я напомнил проблему OPs со следующим примером кода:

enum DataType { Float, Double };

struct Data {
  std::vector<float> dataFloat;
  std::vector<double> dataDouble;
  DataType type;

  Data(const std::vector<float> &data): dataFloat(data), type(Float) { }
  Data(const std::vector<double> &data): dataDouble(data), type(Double) { }
};

При использовании шаблона функции обработка может выглядеть следующим образом:

namespace {

// helper function template for process()
template <typename T>
std::pair<double, double> getMinMax(const std::vector<T> &values)
{
  assert(values.size());
  double min = values[0], max = values[0];
  for (const T &value : values) {
    if (min > value) min = value;
    else if (max < value) max = value;
  }
  return std::make_pair(min, max);
}

} // namespace

void process(const Data &data)
{
  std::pair<double, double> minMax;
  switch (data.type) {
    case Float: minMax = getMinMax(data.dataFloat); break;
    case Double: minMax = getMinMax(data.dataDouble); break;
  }
  std::cout << "range: " << minMax.first << ", " << minMax.second << '\n';
}

Демонстрация в реальном времени на coliru

С макросом это будет выглядеть еще более компактно:

void process(const Data &data)
{
  std::pair<double, double> minMax;
  switch (data.type) {
#define CASE(TYPE) \
    case TYPE: { \
      assert(data.data##TYPE.size()); \
      minMax.first = minMax.second = data.data##TYPE[0]; \
      for (const double value : data.data##TYPE) { \
        if (minMax.first > value) minMax.first = value; \
        else if (minMax.second < value) minMax.second = value; \
      } \
    } break
    CASE(Float);
    CASE(Double);
#undef CASE
  }
  std::cout << "range: " << minMax.first << ", " << minMax.second << '\n';
}

Демонстрация в реальном времени на coliru

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

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

Я бы предпочел третий вариант, который помещает повторяющийся код внутри process(). Лямбда пришла мне в голову, но лямбды не могут (пока) быть шаблонными: ТАК: Могут ли быть лямбда-функции шаблонными? .

Локальный шаблон (функтор) не является альтернативой. Это также запрещено: SO: Почему шаблоны не могут быть объявлены в функции? .


После обратной связи с OP, примечание о X макросах : Это древняя техника в C, предотвращающая избыточность данных.

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

Для использования таблицы данных:

  1. определить макрос X, который использует только те аргументы, которые необходимы в каждом конкретном случае (и игнорирует остальные)
  2. #include таблица данных
  3. #undef X.

Образец снова:

void process(const Data &data)
{
  std::pair<double, double> minMax;
  switch (data.type) {
#define X(TYPE_ID, TYPE) \
    case TYPE_ID: { \
      assert(data.data##TYPE_ID.size()); \
      minMax.first = minMax.second = data.data##TYPE_ID[0]; \
      for (const double value : data.data##TYPE_ID) { \
        if (minMax.first > value) minMax.first = value; \
        else if (minMax.second < value) minMax.second = value; \
      } \
    } break;
#include "Data.inc"
#undef X
  }
  std::cout << "range: " << minMax.first << ", " << minMax.second << '\n';
}

, где Data.inc:

X(Float, float)
X(Double, double)
X(Int, int)

Живой демон на колиру

Хотя эта макро-хитрость немного пугает - ndash; это очень удобно в отношении обслуживания. Если необходимо добавить новый тип данных, то все, что необходимо, - это новая строка X() в Data.inc (и, конечно, перекомпиляция). (Надеемся, что в цепочке компилятор / сборка будут рассмотрены все зависимости исходных кодов от Data.inc. Мы никогда не сталкивались с проблемами в Visual Studio.)

...