undoframework

[SASA]

Смотрю пример undo/redo из примеров qt.
И вот что заметил: DiagramItem никогда не удаляются. Это приводит к утечкам памяти. Добавили сто итемов на, сделали сто откатов, добавили ещё один. Всё, сто итемов повисли в памяти.

Почему возник этот вопрос.
Я реализую undo/redo. При изменении свойств объекта в стеке команд я сохраняю адрес объекта и что поменялось. При удалении объекта я его удаляю физически (delete). Т.е. адрес в стеке становится невалидным. Вот и решил посмотреть как решают эту проблему в примерах Qt. А они оказываются мухлюют - не удаляют объект физически.

А как Вы решаете эту проблему? Или может знаете где посмотреть.

[asvil]

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

[SASA]

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

Не понял...

[Igors]

Ну в стек undo можно записать "сериализованный" объект и при выполнении undo он создастся. Др. дело если указатель на объект хранят др. объекты - вот тут уже "ягодки". Мне известны 2 решения

1) Простое но неряшливое - писать в стек undo всех кто на него ссылался (как бы неск шагов undo которые будут в 1 откате)

2) По уму - хранить ID/handle вместо указателей. Undo/Redo получается очень красиво/естественно. Так напр сделано в 3ds (во всяком случае было). Вот правда сделать это весьма непросто  

[SASA]

Др. дело если указатель на объект хранят др. объекты - вот тут уже "ягодки".

Вот эти ягодки я собираю.

2) По уму - хранить ID/handle вместо указателей. Undo/Redo получается очень красиво/естественно. Так напр сделано в 3ds (во всяком случае было). Вот правда сделать это весьма непросто

Т.е. у всех объектов должно быть id. Но надо иметь способ быстро найти объект с нужным id. В моём случае это не просто, так как структура объектов витвиста и запутана.

А можно подробнее про первый вариант.

[SASA]

Ну в стек undo можно записать "сериализованный" объект и при выполнении undo он

При удалении я и кладу в стек "сериализованный" объект. Но после отмены этой операции я его восстанавливаю, НО  адрес у него другой.

[Igors]

При удалении я и кладу в стек "сериализованный" объект. Но после отмены этой операции я его восстанавливаю, НО  адрес у него другой.

Об этом и речь 

А можно подробнее про первый вариант.

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

1) Вычеркиваете его из таблицы, сначала записав в undo индекс(ы) удаляемых строк. Так придется делать для всех "ссылающихся"
2) Записываете объект(ы) в undo и удаляете

При выполнении undo "задом наперед" - читаете объекты, затем читаете индексы где они стояли в таблице и вставляете

[_OLEGator_]

можно воспользоваться умным указателем:
QSharedPointer, QWeakPointer

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

[SASA]

удалять объекты физически нельзя, они

А надо. Проблема в том, что объекты очень тесно связаны друг с другом и живут своей жизнью. Можно, конечно, сделать для объекта состояние "заморозка". В нём бы рвались все связи. Но многие связи создаются при создании объекта или по какому-то событию. И восстановить их после разморозки просто не реально.

[Igors]

удалять объекты физически нельзя, они

А надо.

Не так просто решить "а надо ли". Дело не только в том что расход памяти постоянно растет - это можно пережить в какой-то момент спросив пользователя "продолжим без undo?". Гораздо хуже просто отсутствие нормального delete и деструкторов - начинаются бесконечные извращения.

С др. стороны удаляя объекты Вы берете на себя (возможно) слишком много. Нормальная реализация требует капитальной переделки всей задачи (что в планы не входило). Иначе придется писать довольно много уродливого кода и пополнять его при каждой новой связке объектов.

В общем SASA не повезло - попал на то что не списывается из Assistant и примеров 

[_OLEGator_]

Значит не правильно спроектировано приложение.

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

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

[Igors]

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

А что "нормального" в таком проектировании? Что делать вместо delete? Что если рассчитываем на удаление членов класса? (а обычно так). Как освобождать ресурсы?

Видел такую "промежуточную" реализацию: объекты честно удаляются, но каждый объект имеет уникальное ID, которое не сохраняется от одной сессии (запуска приложения) до другой, но в течение сессии уникально. Сделано исключительно для undo, удобнее восстанавливать связки между объектами

[_OLEGator_]

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

Итак, есть объекты, есть связи. Пользователь захотел удалить объект из вашей модели. Сейчас физически нельзя удалять объект, т.к. потеряются все сложные связи и указатель в предыдущих операциях отмены/повтора станет не валидным.
Что делается - объект убирается из модели, можно завести флаг о том что надо его игнорировать или еще что-то, но связи с остальными объектами сохраняются. Сам указатель передается в наследник класса QUndoCommand, который при отмене удаления просто вернет указатель в модель, сбросит флаги, а связи сохранятся.
Остается последняя проблема - как физически удалить этот указатель? Удалять физически объект, будет наследник класса QUndoCommand в своем деструкторе, но только в том случае, если не было операции отмены. Деструктор сработает при переполнении стека команд либо при самостоятельном удалении команды отмены/повтора.

Та же самая логика при отмене/повторе создания объекта.

[SASA]

to _OLEGator_. Вы все правильно говорите. Но Вашем решение работает для простых систем.  

можно завести флаг о том что надо его игнорировать

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

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

Ещё есть маленькая проблема в памяти. (большие объекты)

Видел такую "промежуточную" реализацию: объекты честно удаляются, но каждый объект имеет уникальное ID, которое не сохраняется от одной сессии (запуска приложения) до другой, но в течение сессии уникально. Сделано исключительно для undo, удобнее восстанавливать связки между объектами

При таком подходе надо решить следующие проблемы:
- где и кто назначает id
- как их сериализовать
- как искать объект нужным id
Очень красивое решение, но надо написать много кода

Сейчас сделано так.
Вместе с данными в команде для отката сохраняется список указателей. Этот список указателей представляет собой обход дерева объектов, вершина которого, удаляемый объект. Это "старый" список. При откате я создаю объект заново. По сохранённым данным создаётся всё дерево объектов. Потом, получаю "новый" список. Т.к. дерево такое же, алгоритм обхода тот же, то можно точно сопоставить адрес старого объекта и нового. Дальше во всём стеке undo заменяем старый указатель на новый.
Чем мне это не нравиться. Это как-то сложно, много работы с указателями, должна быть гарантирована изоморфность деревьев (старого и нового). Вот я ищу альтернативы.

[Igors]

Сейчас сделано так.
Чем мне это не нравиться. Это как-то сложно, много работы с указателями, должна быть гарантирована изоморфность деревьев (старого и нового). Вот я ищу альтернативы.

Я шел по примерно такому же пути (детали не важны) - и в конце концов понял что это хана   В любом случае - если мы решили удалять объект (а не держать его уродом) надо писать в undo только ID (число) - никогда не указатель.

При таком подходе надо решить следующие проблемы:
- где и кто назначает id
- как их сериализовать
- как искать объект нужным id
Очень красивое решение, но надо написать много кода

Ну это легкота. Базовый класс

Код

C++ (Qt)
typedef long long TID;
class MyObjectID {
public:
  MyObjectID( void )
  {
    mID = ++theGobalID;
    theGlobalMap[mID] = this;
  } 
 
  virtual ~MyObjectID( void )
  {
    TMapID::iterator it = theGlobalMap.find(mID);
    if (it != theGlobalMap.end())
      it->second = 0;
  } 
 
  static MyObjectID * GetByID( TID ID )
  {
    TMapID::iterator it = theGlobalMap.find(ID);
    return (it == theGlobalMap.end()) ? 0 : it->second;
  }
 
private;
  TID mID;  
 
  static TID theGlobalID;
  typedef std::map <TID, MyObjectID *> TMapID;
  static TMapID theGlobalMap:
};
 
// сериализация
QDataStream & QDataStream::operator << ( QDataStream & out, const MyObjectID & );
QDataStream & QDataStream::operator >> ( QDataStream & out, MyObjectID & );
 

Другое дело что ID само по себе еще не решает проблемы поиска "ссылающихся". Это обязательно для undo, но есть и без него. Пример

Код

C++ (Qt)
struct A {
  B * mB;  // A хранит указатель на B
};
 

B может быть удален, но необязательно это делает A, и указатель (mB) надо обнулить. В таких случаях успешно забывается все что прочитано в умных книгах и просто лепится "плохо-код"  "Полная" реализация ID позволяет решить это

Код

C++ (Qt)
struct A {
  TID mB;  // A хранит ID B
};
 

Когда B удален mB остается каким было, просто от него нельзя взять объект. Undo также ничего не делает с A, просто объект уже есть. Развив класс MyObjectID можно сделать так чтобы при удалении/восстановлении "ссылающимся" посылались сигналы чтобы напр они могли перерисовать UI

Ну это "по полной" - а можно ограничиться ID только в целях undo