On Sat, Feb 26, 2011 at 2:52 AM, Dave Stubbs <[email protected]> wrote:
> Yeah, what we have is a nested and OO implementation of a simple stack
> with markers. This allows you to do cool stuff like encapsulate
> complex actions without bothering the user with extra undo steps or
> adding lots of boiler plate code around the "RecordAction" stage.
Don't get me wrong, I think the implementation is fantastic. The only
thing I'm commenting on is some of the unintuitive naming and
mechanics.
For example, passing a function to an operation, where the function
represents a storage operation for an undoable action...is not very
intuitive. In practice, exactly one of two things gets passed to those
functions: either MainUndoStack.getInstance().push, or some
CompositeUndoableAction.addAction. It would therefore be possible to
replace declarations like this:
public function createWay(tags:Object, nodes:Array,
performCreate:Function):Way {
var way:Way = new Way(nextNegative, 0, tags, true, nodes.concat());
performCreate(new CreateEntityAction(way, setWay));
...
with
public function createWay(tags:Object, nodes:Array, actionStack:
CompositeUndoableAction=null):Way {
var way:Way = new Way(nextNegative, 0, tags, true, nodes.concat());
recordAction(actionStack, new CreateEntityAction(way, setWay));
...
where recordAction is:
protected function recordAction(stack: CompositeUndoAbleAction,
action: Action):void {
if (stack)
stack.push(action)
else
MainUndoStack.getInstance().push(action);
}
Personally, I think passing either a composite action stack, or null,
to an operation is a lot more intuitive than passing a function. And
it would make some simpler:
w = createWay({}, nodes);
Now, the fact that it's creating an action which is being placed on
the main undo stack is totally invisible. Magic behind the scenes, and
all that.
Steve
_______________________________________________
Potlatch-dev mailing list
[email protected]
http://lists.openstreetmap.org/listinfo/potlatch-dev
