Refactoring: Extract Class
От: Аноним  
Дата: 10.11.04 04:18
Оценка:
Добрый день!

Вот имеется у меня в проекте класс, реализующий управление неким технологическим объектом. Класс содержит в себе данные о состоянии объекта в целом и о состоянии контуров управления, которые так же реализованы в этом классе.
Что-то типа:
typedef struct
{
    int par1;
    //.......
} COMMON_DATA;

typedef struct
{
    int par1;
    //.......
} ALG_ONE_DATA;

typedef struct
{
    int par1;
    //.......
} ALG_TWO_DATA;

class ControlObject
{
public:
//.................
    void Routine();

private:
    COMMON_DATA        common;    
    ALG_ONE_DATA        alg1;
    ALG_TWO_DATA        alg2;
    // .........................        ........

    void RoutineAlg1();
    void RoutineAlg2();
    //............................
};


Кол-во контуров управления постепенно увеличивалось, их реализация усложнялась. В итоге класс получился довольно тяжелым. Полистав известную книгу господ Фаулера и Бекка, захотелось немного причесать код. А конкретно, выделить каждый контур управления в отдельный класс:
class AlgOne;
class AlgTwo;
//................

class ControlObject
{
public:
//.................
    void Routine()
    {
        pAlg1->Routine();
        pAlg2->Routine();
        //.....................
    }

private:
    COMMON_DATA    common;    
    
    AlgOne    *pAlg1;
    AlgTwo    *pAlg2;

    // .........................        ........

};

class AlgOne
{
public:
    AlgOne( ControlObject *_owner);

    void Routine();

private:
    ALG_ONE_DATA    alg1;

    ControlObject    *owner;
};

class AlgTwo
{
//..................
};


Все вроде нормально, но вот только в исходном классе (ControlObject) была такая фича, как доступ ко внутренним переменным класса по их Id. Просто карта параметров (id, адрес параметра):
{  1, &common.par1},
{  2, &common.par2},
//.........................
{ 10, &alg1.par1},
//.........................
{ 20, &alg2.par1},
//.........................


После рефакторинга те параметры, которые "переехали" в новые классы ес-но стали недоступны таким образом.
На ум приходит несколько вариантов:

1. (Некрасивый) сделать данные public.
2. (Чуть менее некрасивый) объявить классы друзьями (friend)
3. (Не очень удобный) реализовать свою карту параметров в каждом классе. Неудобства — параметры разбросаны по разным местам, соот-но труднее следить чтобы не пересеклись их Id, да и просто листать куду файлов вместо одного неохота.
4. (Нелогичный) Вернуть все данные обратно в (ControlObject). Но ведь они по сути должны "жить" в своих "домиках".

Ну и наконец 5-й вариант. Вариант, который надеюсь предложите Вы, красивый, удобный и логичный

Заранее спасибо.
Re: Refactoring: Extract Class
От: Аноним  
Дата: 11.11.04 02:29
Оценка:
Хм, может я веткой ошибся или непонятно сформулировал проблему.
Re[2]: Refactoring: Extract Class
От: A.J. Россия CintaNotes
Дата: 11.11.04 11:17
Оценка:
Здравствуйте, Аноним, Вы писали:

А>Хм, может я веткой ошибся или непонятно сформулировал проблему.


Как насчет того, чтобы перенести методы, которые интенсивно используют эти атрибуты (данные), в классы, которые имеют эти атрибуты (данные)?
Или вынести эти методы в общий базовый класс, а данные получать через виртуальные set/get?
Re[3]: Refactoring: Extract Class
От: Аноним  
Дата: 11.11.04 13:20
Оценка:
Здравствуйте, A.J., Вы писали:

AJ>Как насчет того, чтобы перенести методы, которые интенсивно используют эти атрибуты (данные), в классы, которые имеют эти атрибуты (данные)?

Именно так я и сделал. Сначала все было в одной куче, много данных и много методов, все в одном классе. Затем, логически сгруппированные методы переехали в собственные классы, соот-но прихватив с собой данные, которые они интенсивно юзают. И если раньше я мог получить любой параметр через:
struct PAR_MAP_ENTRY
{
  int   parId;
  void  *pVar;
};

const PAR_MAP_ENTRY ControlObject::parMap[] =
{
  {  1, &common.par1},
  {  2, &common.par2},
  //.........................
  { 10, &alg1.par1},
  //.........................
  { 20, &alg2.par1}
};

ControlObject::GetParam( int parId, void *pVal)
{
  // Ищем свой параметр в parMap[] и достаем значение по адресу
}

то теперь я могу такое делать лишь с оставшимися данными. Чужие просто в карту нельзя добавить, т.к. они private.


AJ>Или вынести эти методы в общий базовый класс, а данные получать через виртуальные set/get?

"Эти" это какие?

Если получать через вирт. set/get, то их надо реализовывать в каждом классе (и соот-но городить карту параметров тоже в каждом классе). Получается вариант 3 опять же.
Или Вы имели ввиду что-то другое?
Re[4]: Refactoring: Extract Class
От: A.J. Россия CintaNotes
Дата: 11.11.04 14:02
Оценка:
Здравствуйте, Аноним, Вы писали:

Предлагаю на "ты", несмотря на автоматическое "Вы писали", тут принято /местный парадокс/

Не уверен что правильно понял, но все-таки...

А>Именно так я и сделал. Сначала все было в одной куче, много данных и много методов, все в одном классе. Затем, логически сгруппированные методы переехали в собственные классы, соот-но прихватив с собой данные, которые они интенсивно юзают.


Что значит "логически сгруппированные методы"? Класс должен представлять собой какую-то абстракцию. У тебя, насколько я понимаю, есть класс, который чем-то управляет — СontrolObject. Есть контуры управления (выделены как AlgOne, AlgTwo и т.д.). Работа контуров зависит от каких-то параметров. Непонятно, то ли это параметры управляемого объекта, то ли это параметры самих контуров.

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


А>И если раньше я мог получить любой параметр через:

..
А>то теперь я могу такое делать лишь с оставшимися данными. Чужие просто в карту нельзя добавить, т.к. они private.

Если тебе нужна какая-то информация от объекта, делай ее доступной через get-метод.


AJ>>Или вынести эти методы в общий базовый класс, а данные получать через виртуальные set/get?

А>"Эти" это какие?
Которые любят к параметрам обращаться.

А>Если получать через вирт. set/get, то их надо реализовывать в каждом классе (и соот-но городить карту параметров тоже в каждом классе). Получается вариант 3 опять же.


Почему бы не вынести все, что "будет в каждом классе", в базовый класс? тогда в классах, представляющих конкретные реализации контуров, надо будет только определить то, что отличается. И никакого дублирования кода.

Например, как в паттерне Template Method. Или еще можно посмотреть в сторону паттерна Strategy.

Lyric: А вообще, никак без карты параметров нельзя обойтись? зачем она вообще нужна? Разве это задача ControlObject, знать параметры каждого контура? Нельзя сделать что-то вроде
aControlObject->getAlg(0)->getParam(1)?
Re[5]: Refactoring: Extract Class
От: Аноним  
Дата: 11.11.04 15:45
Оценка:
Здравствуйте, A.J., Вы писали:

AJ>Предлагаю на "ты"...

Согласен.

AJ>Что значит "логически сгруппированные методы"? Класс должен представлять собой какую-то абстракцию. У тебя, насколько я понимаю, есть класс, который чем-то управляет — СontrolObject. Есть контуры управления (выделены как AlgOne, AlgTwo и т.д.). Работа контуров зависит от каких-то параметров. Непонятно, то ли это параметры управляемого объекта, то ли это параметры самих контуров.

Все так.
Логически сгруппированные методы — это методы, которые совместно реализуют некий алгоритм управления отдельным контуром (с заданной периодичностью включают исполнительные механизмы, отслеживают изменение параметров объекта, выдерживают определенные временные интервалы).
Насчет параметров. Есть общие параметры (уставки, значения измеренных величин) которые используются всеми/несколькими контурами управления. Кроме этого, каждый контур управления имеет свой набор параметров, которые определяют состояние исключительно этого контура (таймера, счетчики, внутренние режимы работы).

AJ>Нужно сначала анализ предметной области провести, что как с чем соотносится. Тогда все намного проще станет. Проблема в том, что ты пытаешься программу, написанную в процедурном стиле, перевести в ОО-программу без анализа сущностей и назначения ответственностей.

Пытаюсь, это верно. Ответственность ControlObject — обеспечение согласованной работы всех контуров управления и инкапсуляция в себе состояния самого объекта. Ответственность AlgOne, AlgTwo ..... — обеспечение работы конкретных контуров управления.
Отношение м-у ControlObject и AlgOne(Two,...) один к одному. Один ControlObject имеет один контур AlgOne, один контур AlgTwo, .....
Alg* не связаны между собой общим предком и не наследуют от ControlObject.

AJ>Почему бы не вынести все, что "будет в каждом классе", в базовый класс? тогда в классах, представляющих конкретные реализации контуров, надо будет только определить то, что отличается. И никакого дублирования кода.

Потому что инстансы же разные. А каждому классу нужно использовать вполне конкретный экземпляр другого класса.

AJ>Lyric: А вообще, никак без карты параметров нельзя обойтись? зачем она вообще нужна? Разве это задача ControlObject, знать параметры каждого контура? Нельзя сделать что-то вроде

AJ>aControlObject->getAlg(0)->getParam(1)?
Какова будет реализация getParam(1)? Ведь нужно в соответсвие "1" привести какой-то конкретный параметр. Не switch-ем же перебирать.
С картой параметров очень удобно "вынимать" значения параметров, зная только их Id.

Грубо говоря, с верхнего уровня приходит запрос "дай мне значение параметра N 67". Вот я и должен иметь возможность любому параметру (как общему, так и параметру какого-то конкретного контура) назначить этот номер и по запросу без лишнего геммороя достать его значение.
Вот завел я в каком-нибудь из контуров новый параметр, дал ему Id и теперь хочу на ВУ системы (без переделок самого ВУ) поглядеть этот параметр. Ввел я в поле его Id нажал кнопочку и улетел запрос. Как это лучше сделать если параметры разбросаны по классам?
Re[6]: Refactoring: Extract Class
От: Аноним  
Дата: 11.11.04 16:03
Оценка:

Может так чуть яснее станет
Re[6]: Refactoring: Extract Class
От: A.J. Россия CintaNotes
Дата: 12.11.04 19:21
Оценка:
Здравствуйте, Аноним, Вы писали:

А>Вот я и должен иметь возможность любому параметру (как общему, так и параметру какого-то конкретного контура) назначить этот номер и по запросу без лишнего геммороя достать его значение.


Ты хочешь сказать у тебя параметры еще и динамические (т.е. во время работы программы кол-во параметров может меняться?).

А>Вот завел я в каком-нибудь из контуров новый параметр, дал ему Id и теперь хочу на ВУ системы (без переделок самого ВУ) поглядеть этот параметр. Ввел я в поле его Id нажал кнопочку и улетел запрос. Как это лучше сделать если параметры разбросаны по классам?


А!!! Вот теперь понял. Могу предложить такой вариант..
1) В контурах делаешь методы add/remove/has/get/setParam(id)
2) Делаешь класс CommonParams, обязанность которого — предоставлять доступ к общим параметрам, и в нем тоже методы add/remove/has/get/setParam(id). В принципе, если у тебя всего один набор общих параметов, его лучше сделать синглтоном.
3) Из ВУ в ControlObject приходит запрос на добавление нового параметра в контур. Этот запрос приходит с помощью вызова метода addParamToAlg(algName, paramId, paramValue). Объект ControlObject находит нужный Alg и добавляет к нему параметр c помощью вызова addParam(paramId, paramValue).
4) Из ВУ приходит запрос на добавление нового общего параметра, controlObject->addCommonParam(paramId, paramValue). Объект ControlObject вызывает CommonParams::add(paramId, paramValue).
5) Из ВУ приходит запрос на получения значения параметра с id=67. Этот запрос приходит с помощью вызова метода controlObject->getParamById(67). Объект ControlObject итерирует через все контуры и в CommonParams, вызывая у каждого hasParam(67) пока не будет возвращенно TRUE. Потом у этого контура вызывается getParam(67) и значение возвращается в ВУ.

Должно работать
Re[7]: Refactoring: Extract Class
От: A.J. Россия CintaNotes
Дата: 12.11.04 21:01
Оценка:
Как видишь получилась вариация твоего третьего варианта, с вытекающими (описанными тобой) неудобствами.
В следующем дизайне я постарался свести эти неудобства к минимуму:



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

Остается неудобство, что надо следить, чтобы id не пересекались.
Это можно делать в одном месте — у controlObject в addParamToAlg()
(на диаграмме этот метод не показан). Это должен делать controlObject, т.к. только он имеет доступ ко
всем контурам и общим параметрам.

Не уверен, что верно изобразил использование синглтона CommonParams (в виде зависимости).

И еще, я бы серьезно подумал, чтобы разделить controlObject на два — один контроллер и один представляющий состояние контролируемого объекта.
Re[8]: Refactoring: Extract Class
От: A.J. Россия CintaNotes
Дата: 12.11.04 21:14
Оценка:
В принципе, если покажется что этот дизайн ненужно гибкий (например, он позволяет иметь больше одного Alg1,2 и т.д.) можешь его упростить по своим нуждам (например сделать Alg1,2 прямыми потомками ParameterManager и установить соотношение между ними и СontrolObject как 1:1)
Re[7]: Refactoring: Extract Class
От: Аноним  
Дата: 13.11.04 13:59
Оценка:
Здравствуйте, A.J., Вы писали:

AJ>Ты хочешь сказать у тебя параметры еще и динамические (т.е. во время работы программы кол-во параметров может меняться?).

Для НУ системы (речь идет о распределенной АСУТП) кол-во параметров не меняется (смысл динамически заводить там новые параметры, если мы не знаем что потом с ними делать?). А собственно все, о чем шла речь выше это НУ.
А вот для ВУ системы помимо well-known параметров должна быть возможность получать доступ к заранее неизвестным параметрам. Т.е. для ВУ параметры действительно являются динамическими.

AJ>2) Делаешь класс CommonParams, обязанность которого — предоставлять доступ к общим параметрам, и в нем тоже методы add/remove/has/get/setParam(id). В принципе, если у тебя всего один набор общих параметов, его лучше сделать синглтоном.

Один набор общих параметров для одного экземпляра ControlObject (ну и его Alg-ов). На самом дела программа управляет более чем одним объектом (несколько экземпляров ControlObject) и singleton здесь не нужен.

AJ>Как видишь получилась вариация твоего третьего варианта, с вытекающими (описанными тобой) неудобствами.

Получается что так

AJ>В следующем дизайне я постарался свести эти неудобства к минимуму:

В общем-то именно такая картина у меня в голове и вырисовывалась. Приятно, что не только у меня

AJ>Всё что относится к управлению параметрами, вынесено в отдельный абстрактный класс ParameterManager.

Либо так, либо как альтернатива, делать конкретный ParameterManager и "заливать" туда информацию о всех параметрах к которым мы хотим иметь доступ по их Id. В этом случае избавляемся от необходимости итерировать Alg-и для поиска параметра. Соот-но отношение наследования меняется на однонаправленные ассоциации.

AJ>И еще, я бы серьезно подумал, чтобы разделить controlObject на два — один контроллер и один представляющий состояние контролируемого объекта.

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


P.S. Спасибо за конструктивный диалог!
Re[8]: Refactoring: Extract Class
От: A.J. Россия CintaNotes
Дата: 13.11.04 15:41
Оценка: 1 (1)
Здравствуйте, Аноним, Вы писали:

AJ>>2) Делаешь класс CommonParams, обязанность которого — предоставлять доступ к общим параметрам, и в нем тоже методы add/remove/has/get/setParam(id). В принципе, если у тебя всего один набор общих параметов, его лучше сделать синглтоном.

А>Один набор общих параметров для одного экземпляра ControlObject (ну и его Alg-ов). На самом дела программа управляет более чем одним объектом (несколько экземпляров ControlObject) и singleton здесь не нужен.

Тогда да — у каждого ControlObject будет свой CommonParams.

А>P.S. Спасибо за конструктивный диалог!


Рад помочь

Еще маааленький совет: пользователям с именами приятнее отвечать, поэтому они получают больше откликов и получают их быстрее...
Re[9]: Refactoring: Extract Class
От: dream_cast Россия  
Дата: 13.11.04 16:12
Оценка:
Здравствуйте, A.J., Вы писали:

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


Аноним это я был.
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.