C++Talk.NET

Martin · Guest

I am learning C++, and slowly trying to improve my style... I am now
confronted to a design decision that looks like a choice between
Charybde and Scylla, probably because of past bad decisions I made. If
you could help me out of this of state of perplexity, I would be very
grateful.

I want a class (or rather a hierarchy of classes) to implement the
different operations which can be performed on a file. One of its uses
would be inside undo/redo stacks - for an editor. If you think this
design is stupid from the start, please say so Smile

.
So my starting point would be a base abstract class Operation. Each
specific Operation ( Delete, Paste, edition of a field... ) would be
implemented in a derived class.

I also need a way to batch the elementary operations into more complex
ones. The undo/redo stacks will have a "batching state" controlled
through BeginBatch() EndBatch() calls, but I am very hesitant as to
the design of these batches at the level of my Operation class.

I could create a BatchOfOperations class derived from Operation to
implement those (since batches are operations themselves and must be
stored in the undo/redo stack):

First option I see is simply to pass pointer to Operation objects to
the batch, and store them. I have some doubts about the ownership
policy, and how to maintain it though. In what follows the batch takes
ownership of the operation objects. The semantics is not very natural
to say the least, since I end up manipulating pointer to Operation
objects where Operation objects would make more sense.

class Operation
{
public:
virtual ~Operation(){};

/* performs the operation on Basefile& file,
Might invalidate the Operation object when called (make it an
empty operation):
In case of a paste operation for instance, ownership of the
data stored in the operation might actually have been
transferred to the file for the sake of optimization, and so
the operation should not be able to touch it anymore.
*/
virtual operator()(BaseFile & file) = 0;

protected:
virtual void AddTo(BatchOfOperations* batch)
{
batch->Push(this);

}
private:
//non copyable because the semantics don't fit.
Operation(Operation&);
Operation& operator=(Operation&);
};
class BatchOfOperations : public Operation
{

/* each BatchOfOperations stores pointers to the Operations it
groups, it is responsible for deleting them in the destructor,
or possibly when operator() is called */

explicit BatchOfOperations(Operation*& op)
{
Add(op);
}

/* adding Operation *op to our batch
the batch takes ownership of the Operation object op:
*/
void Add(Operation*& op)
{
op->AddTo(this);
op = 0;// make sure this pointer won't be used again
}

protected:
void AddTo(BatchOfOperations& batch)
{
batch.Combine(this);
}
private:

/* adds op to the pointers we store
*/
void Push(Operation* op);

/* retrieves the pointers to Operation objects stored in op
and add them to the ones we store.
Considering our ownership policy should at least empty op of
all its pointers.
This is one of the places where I am uncomfortable with the
ownership policy: Should I assume that op has been dynamically
allocated and simply delete it. This seem in accordance with
the semantics, and furthermore safe: Since void
Add(Operation*& op) takes a non const reference to a pointer,
it guaranties that the object pointed to by op cannot have
automatic lifespan, right? Or I am utterly lost and making that
up?
*/
void Combine(BatchOperation* op);

};

All in all I find the semantics fragile at first view and somewhat
unnatural, can something like that be made dependable and acceptable?
It seems

A variant would be to clone every Operations I want to store in the
batch, the result is much better semantically I think, but I am going
to have problems behind the scene.
The data needed to perform certain operation is extensive, a paste
operation for instance will have to store the piece of the file to
paste (and the files the editor is manipulating are relatively big,
typically 10 or 20 Mo of data). If I want to maintain the integrity of
every copy of an Operation object, I'll have to either duplicate that
data (not really an option in this context) or keep a reference count
on it. The same is true when calling operator()(Basefile&) instead of
simply transferring ownership of the data, I will have to use a
reference count, that will necessitate a big rewriting of associated
classes... In the end I wonder if I will not make things more
complicated than they should be (especially considering I am almost
sure there are some problems I am not seeing yet) for a limited
benefit.

class Operation
{
public:
virtual ~Operation(){};
virtual operator()(BaseFile &) = 0; // performs the operation
on Basefile
protected:
virtual void AddTo(BatchOfOperations* batch)
{
batch->Push(this);
}
virtual Operation* Clone() = 0;
};

class BatchOfOperations : public Operation
{

/* each BatchOfOperations can safely delete the Operation */

explicit BatchOfOperations(Operation*& op)
{
Add(op);
}

/* adding Operation op to our batch*/
BatchOfOperations& Add(const Operation& op)
{
Operation* clone = op.Clone();
clone->AddTo(this);
}

protected:
void AddTo(BatchOfOperations& batch)
{
batch.Combine(this);
}
private:

/* adds op to the pointers we store
*/
void Push(Operation* op);

/* retrieves the pointers to Operation objects stored in op
and add them to the ones we store.
Considering our ownership policy should empty op of all its
pointers (make it an empty batch) */
void Combine(BatchOperation* op);

};

An other approach would be to discard the BatchOfOperations class
altogether, and instead make every operation a potential "starting
point" for a batch something like:

class Operation
{
operator()(Basefile& file)
{
ApplyTo(file);
if(nextOp_)
{
(*nextOp_)(file);
}
}
private:
Operation* nextOp_;
/* implements the actual operation: */
virtual DoApply(BaseFile*) = 0;
};

But this doesn't really change a thing, I end up with the same
problems and alternative as with the first two options (in terms of
ownership of the operations linked to, data, copy construction...).

Thanks for your advices.

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

Patrick May · Guest

Martin <martin (AT) proctophantasmist (DOT) cjb.net> writes:

kanze · Guest

Martin wrote:

ravinderthakur@gmail.com · Guest

Gerhard Menzl · Guest

Martin wrote:

kanze · Guest

Gerhard Menzl wrote:

[...]