Про рефакторинг
От: andyp  
Дата: 16.01.12 13:30
Оценка:
Здравствуйте,

Есть некая реализация штуковины System из предметной области. Она включает несколько подсистем:

//заголовок system.h
class Subsystem1;
class Subsystem2;
class Subsystem3;

class System
{
    Subsystem1* _s1;        
    Subsystem2* _s1;        
    Subsystem2* _s2;        
public:
    System();
    ~System();
    //интерфейс System 
    void some_method();
    //часть методов используется только subsystem1
    void used_only_by_subsystem1();
    //часть методов используется только subsystem2
    void used_only_by_subsystem2();
};

//реализация методов system.cpp
#include "system.h"
#include "subsystem1.h"
#include "subsystem2.h"
//etc...
System::System()
{
    //создаем подсистему
    _s1 = new Subsystem1; //аналогично для Subsystem1, Subsystem2, ...
}

void System::some_method()
{
    //используем ее
    _s1->some_method();    
}

System::~System()
{
    //убираем за собой
    delete _s1;

}

//заголовочный файл subsystem1.h
class System;
class Subsystem1
{
    System* _owner;
public:
    Subsystem1(System* o);
    void some_method();
};

//реализация методов  - файл subsystem1.cpp
#include "subsystem1.h"
#include "system.h"

Subsystem1::Subsystem1(System* o):_owner(o){}
void Subsystem1::some_method()
{
    //...
    //иногда приходится что-то делать с владельцем
    _owner->used_only_by_subsystem1();

}



Получается, что:
1.заголовочные файлы, описывающие подсистемы не включаются в system.h (это хорошо, так как подсистемы — деталь реализации)
2. заголовочный файл системы не включается в заголовочный файл подсистемы (это тоже хорошо, так как интерфейс подсистемы не зависит от системы)

Хотелось бы, сохраняя свойства 1 и 2, как-то убрать методы used_by_subsyst из public секции описания класса System потому что они только замусоривают интерфейс системы,
а по факту используются только при ее реализации. Как этого добиться проще всего не теряя скорости выполнения?
Re: Про рефакторинг
От: night beast СССР  
Дата: 16.01.12 13:48
Оценка:
Здравствуйте, andyp, Вы писали:

A>Здравствуйте,


A>Есть некая реализация штуковины System из предметной области. Она включает несколько подсистем:



A>Получается, что:

A>1.заголовочные файлы, описывающие подсистемы не включаются в system.h (это хорошо, так как подсистемы — деталь реализации)
A>2. заголовочный файл системы не включается в заголовочный файл подсистемы (это тоже хорошо, так как интерфейс подсистемы не зависит от системы)

A>Хотелось бы, сохраняя свойства 1 и 2, как-то убрать методы used_by_subsyst из public секции описания класса System потому что они только замусоривают интерфейс системы,

A>а по факту используются только при ее реализации. Как этого добиться проще всего не теряя скорости выполнения?

pimpl не подойдет?
class System {
    struct SystemImpl;
    auto_ptr<SystemImpl> m_impl;
public:
    System();
    ~System();
    
    void some_method(); //интерфейс System 
};
Re[2]: Про рефакторинг
От: andyp  
Дата: 16.01.12 14:02
Оценка:
Здравствуйте, night beast, Вы писали:

NB>Здравствуйте, andyp, Вы писали:


A>>Здравствуйте,


A>>Есть некая реализация штуковины System из предметной области. Она включает несколько подсистем:



A>>Получается, что:

A>>1.заголовочные файлы, описывающие подсистемы не включаются в system.h (это хорошо, так как подсистемы — деталь реализации)
A>>2. заголовочный файл системы не включается в заголовочный файл подсистемы (это тоже хорошо, так как интерфейс подсистемы не зависит от системы)

A>>Хотелось бы, сохраняя свойства 1 и 2, как-то убрать методы used_by_subsyst из public секции описания класса System потому что они только замусоривают интерфейс системы,

A>>а по факту используются только при ее реализации. Как этого добиться проще всего не теряя скорости выполнения?

NB>pimpl не подойдет?

NB>
NB>class System {
NB>    struct SystemImpl;
NB>    auto_ptr<SystemImpl> m_impl;
NB>public:
NB>    System();
NB>    ~System();
    
NB>    void some_method(); //интерфейс System 
NB>};
NB>


Может и годится. Намекаете на то, что private методы System станут доступны System::SystemImpl ?
Re: Про рефакторинг
От: MasterZiv СССР  
Дата: 16.01.12 14:06
Оценка:
On 01/16/2012 05:30 PM, andyp wrote:

> Хотелось бы, сохраняя свойства 1 и 2, как-то убрать методы used_by_subsyst из

> public секции описания класса System потому что они только замусоривают
> интерфейс системы,
> а по факту используются только при ее реализации. Как этого добиться проще всего
> не теряя скорости выполнения?

Варианты
0) private, friend
1) выделить общение Sustem/Subsystem в систему встречнонаправленных интерфейсов
(Bridge), сами интерфейсы -- private, friend
Posted via RSDN NNTP Server 2.1 beta
Re[2]: Про рефакторинг
От: andyp  
Дата: 16.01.12 14:25
Оценка:
Здравствуйте, MasterZiv, Вы писали:

MZ>Варианты

MZ>0) private, friend

думал так. Грубоват контроль над доступностью методов получается.

MZ>1) выделить общение Sustem/Subsystem в систему встречнонаправленных интерфейсов

MZ>(Bridge), сами интерфейсы -- private, friend

Вы имеете в виду паттерн Bridge? Написать абстрактные интерфейсы System->Subsystem и Subsystem->System, сделать конкретные реализации для System и Subsystem, в которых хранить указатели на соответствующие абстрактные классы?
Re: Про рефакторинг
От: minorlogic Украина  
Дата: 16.01.12 14:30
Оценка:
Виртуальные функции и интерфейсы ?


class System
{
  public:
    ISubsystem1* getSubsystem1();        
    ISubsystem2* getSubsystem2();        
    ISubsystem3* getSubsystem3();        
};
... << RSDN@Home 1.2.0 alpha 5 rev. 1539>>
Ищу работу, 3D, SLAM, computer graphics/vision.
Re: Про рефакторинг
От: andyp  
Дата: 16.01.12 14:56
Оценка: :)
Спасибо всем!

решил остановиться на идиоме pimpl:

class System
{
public:
//...
private:
class Subsystem1;
Subsystem1* _s1; //ну или auto_ptr
}

Единственный на мой взгляд недостаток — все остальные private члены класса видны в Subsystem1, но с этим вполне можно жить. Bridge делать не захотел — овчинка не стоит выделки в моем случае.
Re[3]: Про рефакторинг
От: night beast СССР  
Дата: 16.01.12 15:27
Оценка:
Здравствуйте, andyp, Вы писали:

NB>>pimpl не подойдет?

NB>>
NB>>class System {
NB>>    struct SystemImpl;
NB>>    auto_ptr<SystemImpl> m_impl;
NB>>public:
NB>>    System();
NB>>    ~System();
    
NB>>    void some_method(); //интерфейс System 
NB>>};
NB>>


A>Может и годится. Намекаете на то, что private методы System станут доступны System::SystemImpl ?


намекаю на то что эти private методы можно засунуть в System::SystemImpl.
Re: Про рефакторинг
От: Кодт Россия  
Дата: 16.01.12 15:45
Оценка:
Здравствуйте, andyp, Вы писали:

A>Хотелось бы, сохраняя свойства 1 и 2, как-то убрать методы used_by_subsyst из public секции описания класса System потому что они только замусоривают интерфейс системы,

A>а по факту используются только при ее реализации. Как этого добиться проще всего не теряя скорости выполнения?

Ну казалось бы.
Есть интерфейс System — для всех.
Есть интерфейсы Subsystem1..3 — для System.
Есть интерфейсы Owner1..3 — для Subsystem1..3 соответственно.

Можно решить задачу даже без идиоматеских подходов, а чисто на ООП.
Класс SystemImpl реализует 4 интерфейса — System, Owner1..3.
Пользователи имеют дело с умным указателем на System.
Конструктор класса SystemImpl создаёт объекты SubsystemImpl1..3 с интерфейсами Subsystem1..3, отдавая им указатели на свои интерфейсы Owner1..3 соответственно.

Связку интерфейс-реализация (System-SystemImpl) можно делать силами языка, то есть на виртуальных функциях, а можно своими силами, то есть на идиоме pimpl.
Честно сказать, я не уверен, что pimpl даст реальный выигрыш в скорости (хотя, конечно, там на одну косвенность меньше, чем при вызове виртуальной функции). Надо профилировать.

Связки интерфейсов-реализаций остальных типов можно делать статическими.
Для SubsystemN-SubsystemImplN это тривиально (это один и тот же тип).
А для OwnerN-SubsystemImpl это или будет "всё для всех" (т.е. Subsystem1 будет видеть Owner2, потому что Owner1=Owner2=Owner3=SystemImpl), или придётся использовать идиому CRTP.
А можно и не нахлобучиваться, а сделать и это тоже на виртуальных функциях.

На всякий случай: как это делается на CRTP.
// owner1.h

class SystemImpl;

template<class Impl>
struct Owner1T
{
  void foo() { static_cast<Impl*>(this)->foo_impl(); }
  // без шаблона мы не смогли бы так легко выполнить статический downcast
};

typedef Owner1T<SystemImpl> Owner1; // внимание! шаблон параметризован недообъявленным типом!

// subsystem1.h

#include "owner1.h"

class Subsystem1
{
public:
  Subsystem1(Owner1* owner) : owner_(owner) {}
private:
  Owner1* owner_;
  void call_foo(); // нам обязательно надо видеть объявление SystemImpl
};

// subsystem1.cpp

#include "systemimpl.h"

void Subsystem1::call_foo() { owner_->foo(); }

// systemimpl.h

#include "owner1.h"
#include "subsystem1.h"

class SystemImpl : ....., private Owner1
{
  ....
  void foo_impl(); // либо public, либо надо объявить friend Owner1
};
Перекуём баги на фичи!
Re[3]: Про рефакторинг
От: MasterZiv СССР  
Дата: 16.01.12 16:02
Оценка:
> MZ>Варианты
> MZ>0) private, friend
>
> думал так. Грубоват контроль над доступностью методов получается.

Ну, в общем-то с точностью до конкретных методов, которым доверяют.

>

> MZ>1) выделить общение Sustem/Subsystem в систему встречнонаправленных интерфейсов
> MZ>(Bridge), сами интерфейсы -- private, friend
>
> Вы имеете в виду паттерн Bridge? Написать абстрактные интерфейсы
> System->Subsystem и Subsystem->System, сделать конкретные реализации для System
> и Subsystem, в которых хранить указатели на соответствующие абстрактные классы?

Да, типа того.
Posted via RSDN NNTP Server 2.1 beta
Re[2]: Про рефакторинг
От: MasterZiv СССР  
Дата: 16.01.12 16:08
Оценка:
On 01/16/2012 06:56 PM, andyp wrote:

> решил остановиться на идиоме pimpl:

>
> class System
> {
> public:
> //...
> private:
> class Subsystem1;
> Subsystem1* _s1; //ну или auto_ptr
> }

Это и есть Bridge.
Posted via RSDN NNTP Server 2.1 beta
Re[3]: Про рефакторинг
От: andyp  
Дата: 16.01.12 17:51
Оценка:
Здравствуйте, MasterZiv, Вы писали:

MZ>On 01/16/2012 06:56 PM, andyp wrote:


>> решил остановиться на идиоме pimpl:

>>
>> class System
>> {
>> public:
>> //...
>> private:
>> class Subsystem1;
>> Subsystem1* _s1; //ну или auto_ptr
>> }

MZ>Это и есть Bridge.


Не совсем. Вроде бы у Bridge должна иметь место быть абстракция System и Subsystem1? (по крайней мере так в книжке у Гаммы написано)
Re[2]: Про рефакторинг
От: andyp  
Дата: 16.01.12 17:52
Оценка:
Спасибо за бридж на шаблонах. Буду разбираться.
Re[3]: Про рефакторинг
От: Кодт Россия  
Дата: 16.01.12 18:25
Оценка:
Здравствуйте, andyp, Вы писали:

A>Спасибо за бридж на шаблонах. Буду разбираться.


Там от шаблонов только трюк, чтобы обойти приведение типа во время компиляции. Без шаблона компилятор бы выругался на недообъявленный тип.
А вообще, CRTP — это compile-time аналог виртуальных функций.

Но это не бридж!
Это простое наследование реализации от интерфейса.
Сравни
struct Owner1 { virtual void foo() = 0; };

class Subsystem1 {
  Owner1* owner_;
  void call_foo() { owner_->foo(); }
public:
  Subsystem1(Owner1* owner) : owner_(owner) {}
  void do_something() { call_foo(); }
};

class SystemImpl : Owner1
{
  virtual void foo() { ... };
  .....
};


А уже между интерфейсом System и реализацией SystemImpl — там может быть и бридж.
Перекуём баги на фичи!
Re[4]: Про рефакторинг
От: MasterZiv СССР  
Дата: 16.01.12 18:56
Оценка:
> MZ>Это и есть Bridge.
>
> Не совсем. Вроде бы у Bridge должна иметь место быть абстракция System и
> Subsystem1? (по крайней мере так в книжке у Гаммы написано)

Даже если и так, Это малозначительные детали.
Posted via RSDN NNTP Server 2.1 beta
Re[4]: Про рефакторинг
От: andyp  
Дата: 16.01.12 19:25
Оценка:
Понял, это способ дать Subsystem нужный кусок интерфейса System. Что такое CRTP я представлял, но даже подумать не мог его сюда приткнуть
Re[3]: Про рефакторинг
От: night beast СССР  
Дата: 17.01.12 02:05
Оценка:
Здравствуйте, andyp, Вы писали:

A>Может и годится. Намекаете на то, что private методы System станут доступны System::SystemImpl ?


благодаря пояснению ( спасибо Кодт! ) понял, чего ты хочешь добиться ( может и не правильно )
// system.hpp
class OwnerBase;

class SubSystem1;

class System {
   friend class OwnerBase;
public:
   System ( );

   void some_method ( );

private:
   int value;
   SubSystem1 * m_sub1;
};

// owner.hpp
#include "system.hpp"
class OwnerBase {
   System * m_owner;
public:
   OwnerBase ( System * owner ) : m_owner( owner ) { }

   void some_method ( ) { owner->some_method( ); }

protected: 
   int get_value( ) { return m_owner->value; }
};

class Owner1 : OwnerBase {
public:
   Owner1 ( System * owner ) : OwnerBase( owner ) { }
public: 
   int used_only_by_subsystem1( ) { return this->get_value( );}
};

// subsystem1.hpp
#include "owner.hpp"
class SubSystem1 {
public:
   SubSystem1 ( Owner1 owner ) { std::cout << owner.used_only_by_subsystem1( ); }
};


// system.cpp
#include "system.hpp"
#include "subsystem1.hpp"

System::System ( ) : value ( 10 ), m_sub1( 0 ) { m_sub1 = new SubSystem1( this ); }
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.