LINUX.ORG.RU

С++ Помогите избавиться от дублирования однотипного кода.

 


1

3

Всем доброго времени суток. Возникла проблема (точнее, понимание того, что в одном месте код фактически N раз дублируется). Приведенный далее код просто поясняет проблему и даже не компилировался.

Дано: 1. Интерфейсный класс с тремя (на самом деле, их значительно больше) методами разных сигнатур.

// Интерфейсный класс родителя.
class IParent
{
public:
   virtual bool fu0() = 0;
   virtual bool fu1(const std::string &) = 0;
   virtual std::string fu2() = 0;
   virtual ~IParent() = 0;
};

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

class CChild0 : public IParent
{
public:
   virtual bool fu0() override
   {
      // Что-то делаем.
   }
   virtual bool fu1(const std::string & param) override
   {
     // Опять что-то много делаем.
   }
   virtual std::string fu2() override
   {
      // И тут делаем.
   }
};

3. Супер-класс, который хранит в себе через указатели на базовый класс потомков в мапе.

class CSuperHandler
{
private:
   // Мапа для хранения потомков через указатель на родителя. Ключ - далее id.
   std::map<int, std::unique_ptr<IParent>> mMap;
public:
   CSuperHandler()
   {
      //Заполнение мапы может быть не только тут.
      mMap.insert(std::make_pair(0, std::unique_ptr<IParent>(new CChild0)));
      mMap.insert(std::make_pair(1, std::unique_ptr<IParent>(new CChild1)));
      mMap.insert(std::make_pair(2, std::unique_ptr<IParent>(new CChild2)));
   }
   
   // Должен запускать метод fu0 если найден потомок.
   bool SuperFu0(const int id)
   {
      auto it = mMap.find(id);
      bool returnValue = false;
      if(it != mMap.end())
      {
         returnValue = it->second->fu0();
      }
      else
      {
         std::cout << "ID not found.";
      }
      return returnValue;
   }
   
   // Должен запускать метод fu1 если найден потомок.
   bool SuperFu1(const int id, const std::string & param)
   {
      auto it = mMap.find(id);
      bool returnValue = false;
      if(it != mMap.end())
      {
         returnValue = it->second->fu1(param);
      }
      else
      {
         std::cout << "ID not found.";
      }
      return returnValue;
   }
   
   // Должен запускать метод fu2 если найден потомок.
   std::string SuperFu2(const int id)
   {
      auto it = mMap.find(id);
      std::string returnValue = "";
      if(it != mMap.end())
      {
         returnValue = it->second->fu2();
      }
      else
      {
         std::cout << "ID not found.";
      }
      return returnValue;
   }
};

Собственно, что проблема в том, что методы SuperFu0, SuperFu1,SuperFu2 (а на самом деле, их много) имеют практически одинаковое содержимое: проверяют есть ли нужный id и запускают у соотвествующего потомка метод, результат которого возвращают. Пока я не придумал как можно это дело оптимизировать.

Хотелось бы, конечно, иметь что-то вроде:

template<typename T, typename... Args>
T doAction(int id, std::function<T(const Args & ... args)> fu, const Args & ... args)
{
  auto it = mMap.find(id);
  T returnValue;
  if(it != mMap.end())
  {
    returnValue = it->second->fu(args...);
  }
  else
  {
    std::cout << "ID not found.";
  }
  return returnValue;
} 

Но мы же не можем передать туда указатель на метод объекта, которого при вызове шаблонного метода еще не знаем (он внутри по id определяется). В общем, если кто-то подскажет какое-то удачное решение - буду очень рад. Спасибо!

с++

Тэг кириллицей? По теме: вынеси поиск во внутреннюю функцию и вызывай её из методов суперкласса. Даже шаблонной делать не придётся

Deleted ()
Ответ на: комментарий от Deleted

Можно таки сделать шаблонной что то в духе:

template <typename IfaceT, typename IdT>
IfaceT* dispatch(IdT id) {
// search
// throw on failure
// return
}

void super_foo(IdT id, ArgT arg) {
    return dispatch<Foo>(id)->foo(arg);
}

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

Как по мне, в точке вызова лучше делать просто так:

SuperFoo foo;
// ...
foo.dispatch(id)->call(arg);

pon4ik ★★★★★ ()
Последнее исправление: pon4ik (всего исправлений: 1)
Ответ на: комментарий от pon4ik

Я в код особо сильно не вникал, но в глаза бросается следующее:

  • поиск одного и того же типа по map
  • наличие явного интерфейса
  • id так же одного и того же типа

Ну и как бы самое очевидное и простое решение. В общем автор не особо полно описал, в чём трудность

Deleted ()

Но мы же не можем передать туда указатель на метод объекта, которого при вызове шаблонного метода еще не знаем

Почему нет? Тип объекта же известен, значит можно передать указатель на метод (&IParent::fu0).

xaizek ★★★★★ ()
  • Переделай CSuperHandler в CSuperLocator, пусть он ничего не вызывает а только возвращает указатель на нужный класс. Если класс не найден, можешь возвращать nullptr, кидать, или возвращать указатель на статический экземпляр ничего не делающего/логгирующего/кидающего наследника IParent.
  • Ты можешь сделать doAction(int id, std::function<void(const IParent &)>), которая вызывается на найденном объекте и не вызывается если ничего не найдено.
  • В текущей реализации ты можешь упростить код унаследовав CSuperHandler от IParent (позволит не ошибиться с аргументами и не пропустить методы), а одинаковые тела функций вынести в шаблонный метод, соответственно сократив тела fu0, fu1, fu2 до одной строчки.
slovazap ★★★★★ ()
Ответ на: комментарий от Deleted

Извиняюсь за тег - не обратил внимания.

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

Самое забавное, что это первое о чем я подумал и сейчас в упор не могу вспомнить почему мне это решение не подошло))

bool checkID(const int id)
{
   auto it = mMap.find(id);
   if(it != nMap.end())
   {
      return true;
   }   
   else
   {
      // Do something if invalid ID.
   }
}

bool SuperFu0(const int id)
{
   bool returnValue = false;
   if(checkID(const int id)
   {
      returnValue =  mMap[id].fu0();
   }  
   return returnValue;
}
igniperch ()
Ответ на: комментарий от pon4ik

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

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

igniperch ()
Ответ на: комментарий от Deleted

поиск одного и того же типа по map

Ну, map мне нужен скорее для более быстрого доступа к потомкам. Т.е. мне где-то нужно хранить N разных классов, у которых есть M общих методов.

igniperch ()
Ответ на: комментарий от xaizek

Почему нет? Тип объекта же известен, значит можно передать указатель на метод (&IParent::fu0).

Спасибо. Подумаю)

igniperch ()

SuperFu не должно ничего запускать, только искать и возвращать ссылку/указатель на IParent. Если не находит, то кидать исключение. Тогда ты будешь вместо

superInst->SuperFu0(42);
писать
superInst->findElt(42)->fu0(); // или ->fu1(param) и т.п.

no-such-file ★★★★★ ()
Последнее исправление: no-such-file (всего исправлений: 1)
Ответ на: комментарий от no-such-file

Если тебе по каким-то причинам не хочется использовать исключения, то можно в SuperHandler при инициализации передавать «дефолтный» объект IParent, который будет возвращаться из findElt когда элемент не найден в map. Там можно делать cout и т.п. вещи.

no-such-file ★★★★★ ()
Последнее исправление: no-such-file (всего исправлений: 1)

Но мы же не можем передать туда указатель на метод объекта

А лямбду передать не покатит? А в ней уже вызывать нужный метод у правильного объекта.

у меня завтра свадьба

Мазаль тов. Своя хоть свадьба-то?

anonymous ()
Ответ на: комментарий от no-such-file

SuperFu не должно ничего запускать, только искать и возвращать ссылку/указатель на IParent.

Спасибо, что-то в этом есть.

igniperch ()
Ответ на: комментарий от no-such-file

Если тебе по каким-то причинам не хочется использовать исключения,

Увы, исключения нельзя использовать.

то можно в SuperHandler при инициализации передавать «дефолтный» объект IParent, который будет возвращаться из findElt когда элемент не найден в map.

Хм. Надо подумать.

igniperch ()
Ответ на: комментарий от anonymous

А лямбду передать не покатит? А в ней уже вызывать нужный метод у правильного объекта.

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

igniperch ()
Ответ на: комментарий от anonymous

Мазаль тов. Своя хоть свадьба-то?

Спасибо. Да, моя была. Впрочем, как уже, наверное, понятно, что особых торжественных мероприятий не планировалось)

igniperch ()

Если вдруг кому интересно, решение примерно следующее:

template<class TService
              ,typename ... TRequestParameters
              ,typename ... TParameters
              ,typename ReturnType = typename std::result_of< TService(TRequestParameters...) >::type>
ReturnType CSuperHandler::call(int id, ReturnType (TService::*request)(TRequestParameters ...), TParameters ... parameters)
{
  // Do something.
  return std::mem_fn(request)(mObjects.at( id ), parameters...);
}

Вызывается так:

bool CSuperHandler::SuperFu0(const int id) 
{
   return call(id, &IParent::fu0); 
}

Или так:

bool CSuperHandler::SuperFu1(const int id, const std::string & param)
{
   return call(id, &IParent::fu1, param); 
}
В общем, конечно, трудночитаемо и велосипедновато как-то. Всем спасибо за ответы.

igniperch ()
Ответ на: комментарий от eao197

У вас жесткое ограничение на C++11?

Вообще, относительно недавно не было даже одиннадцатого. Сейчас 14й максимум. Я еще не до конца освоился с новыми реалиями и от того, видимо, много проблем. Будем потихоньку исправляться (увы,слишком часто на самообучение попросту нет времени).

В C++17 std::result_of объявлен как deprecated, цинк.

Спасибо за предупреждение. Я не заметил(( Видимо, придется думать в сторону лямбд.

igniperch ()
Ответ на: комментарий от igniperch

Если вы можете использовать C++14, то там получается компактнее:

template<typename Method, typename... Args>
auto call(int id, Method method, Args && ...args) {
	auto & obj = *(mMap.find(id)->second);
	return (obj.*method)(std::forward<Args>(args)...);
}

Полный пример тут: https://wandbox.org/permlink/tIOKVr3srAeegSFY

eao197 ★★★★★ ()
Ответ на: комментарий от eao197

Если вы можете использовать C++14, то там получается компактнее:

Ого, спасибо! Будет свободное время - обязательно проверю!

P.S. В рамках оффтопа безусловно хочется немного поныть на тему того, что похоже, что редкая смена рабочего места у меня (как и у многих экс- и просто коллег) приводит к застою в развитии. Потому, что долгосрочный проект в итоге сводится к поддержке и развитию существующего legacy-кода, который был написан в незапамятные времена и уже лет пять-семь как требует полного переписывания рефакторинга, на который нет элементарно времени. Нужно же «пилить фичи» и «править баги».

igniperch ()
Ответ на: комментарий от igniperch

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

Но «в каждой избушке свои погремушки», кому как повезет.

eao197 ★★★★★ ()
Ответ на: комментарий от eao197

Но «в каждой избушке свои погремушки», кому как повезет.

У нас расписание появления новых фич расписано до 2024 года (и я не шучу, просто с 2019го уже с детализацией не в две недели, а в месяц, а потом раз в два месяца). Так что максимум что получается выделить под рефакторинг достигается йогой формулировок (например, не «refactoring», а «stability improvement» и т.д.) и просто везением (когда что-то успеваешь сделать быстрее). В общем, еще раз спасибо за помощь и приятный диалог.

igniperch ()
Ответ на: комментарий от eao197

Интересно, это в какой области деятельности такой горизонт планирования?

automotive

igniperch ()
Ответ на: комментарий от deep-purple

От качества кода зависит, в принципе.

Есть очевидные моменты, которые требуют переделки. Например, если активно использовались auto_ptr, то это нужно заменять на unique_ptr.

Но вообще, если код нормальный, то особых проблем нет.

При переходе с 32-х бит на 64-е нужно было быть внимательнее. Не говоря уже про переход с 16-ти на 32 ;)

eao197 ★★★★★ ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.