Futurescale, Inc. PureMVC Home

The PureMVC Framework Code at the Speed of Thought


Over 10 years of community discussion and knowledge are maintained here as a read-only archive.

New discussions should be taken up in issues on the appropriate projects at https://github.com/PureMVC

Pages: [1] 2
Print
Author Topic: Implementing Undoable commands in PureMVC  (Read 36378 times)
justsee
Courseware Beta
Newbie
***
Posts: 9


View Profile Email
« on: October 15, 2007, 07:57:41 »

Hi there,
 I'm interested in people's thoughts on this approach to implementing undoable / redoable commands within a PureMVC application:
1. create an IUndoableCommand interface in controller namespace with methods undo() and redo()
2. create a CommandHistoryProxy as below
:
public class CommandHistoryProxy extends Proxy implements IProxy {

private var _commands:Array;
private var _index:uint;

public static const NAME:String = 'CommandHistoryProxy';

public function CommandHistoryProxy() {
super(NAME);
init();
}

private function init():void {
_commands = new Array();
_index = 0;
}

public function putCommand(command : IUndoableCommand) : void {
_commands[_index++] = command;
_commands.splice(_index, _commands.length - _index);
}

public function previous():IUndoableCommand {
return _commands[--_index];
}

public function next():IUndoableCommand {
return _commands[_index++];
}

public function hasPreviousCommands():Boolean {
return _index > 0;
}

public function hasNextCommands():Boolean {
return _index < _commands.length;
}
}
}
3. create a concrete ItemSelectedCommand command which implements IUndoableCommand. Its execute method does something like this:
:
undo_item = INode(note.getBody());
var command_history_proxy : CommandHistoryProxy = facade.retrieveProxy(CommandHistoryProxy.NAME) as CommandHistoryProxy;
command_history_proxy.putCommand(this);
Its undo and redo methods change state and send appropriate notifications
4. A ComandHistoryMediator which listens for ApplicationFacade.ITEM_SELECTED notifications (to toggle visibility of undo / redo buttons), and contains a CommandHistory view component which:
  • contains undo and redo buttons
  • has public methods to set the visibility of each
  • dispatches CommandHistory.UNDO and CommandHistory.REDO methods
The CommandHistoryMediator defines onUndo and onRedo methods which simply retrieve the appropriate command
:
var command:IUndoableCommand = command_history_proxy.previous() and then calls command.undo / command.redo

I was also wondering what the best approach for implementing multiple arrays of command histories would be? Is the type parameter of the sendNotification method an appropriate place to specify the type of the command history to store? For instance, if CommandHistoryProxy was reworked to store arrays of command histories as a hash, using the sendNotification's state method I might do something like sendNotification(ApplicationFacade.ITEM_SELECTED, item, ApplicationFacade.APPROPRIATE_STATE), so the execute method of ItemSelectedCommand might do
:
command_history_proxy.putCommand(this, ApplicationFacade.APPROPRIATE_STATE
I'm also wondering whether it makes sense to subclass Notification to implement getUndoBody() and getRedoBody() depending on the command history requirements. Is that a sensible case for subclassing?

cheers,
justsee
Logged
puremvc
Global Moderator
Hero Member
*****
Posts: 2871



View Profile WWW Email
« Reply #1 on: October 16, 2007, 03:21:36 »

Justsee,

I've been waiting for the undoable command question to pop up. Not actually waiting, per se, more wondering if it would happen before I had time to address it in a demo. I considered including support for undoable commands in the core distribution,  but I felt it overly complicated the framework and would be best left as an add-on class which people can take or leave as they choose.

My first take on this is that it looks like a workable approach, but it distributes a Controller region responsibility into the Model region. Now, that might just be a virtue if we want to persist the Command history, so I certainly don't want to appear to shoot this approach down. It would be good to look over a working demo using this concept before I could comment much further.

The idea I was envisioning was a Controller subclass (UndoableController, lets say) with it's own internal history cache and undo/redo methods. Its putCommand method would be private and called automatically whenever it executes a Command that implements IUndoable.

This contains the problem in a single subclass. And invoking UNDO/REDO could be as easy as having the UndoableController automatically register its own undo and redo methods as Observers for the UndoableController.UNDO and .REDO Notifications when it is constructed. Then in any Mediator that wants to invoke undo/redo simply sends those Notifications.

-=Cliff>
Logged
ddragosd
Courseware Beta
Jr. Member
***
Posts: 10


View Profile Email
« Reply #2 on: November 30, 2007, 05:05:45 »

Hi,

I got inspired from your ideas and I thought I should post mine, just to continue the discussion you initiated. These days I had to implement an undo/redo system into an application and this is how I did it:

1.   I created an UndoableCommandBase class that extends SimpleCommand. UndoableCommandBase is a superclass for any command within the application that must be undoable. I added 4 more methods.
a.   registerUndoCommand – registers the command that needs to be called on undo

:
/**
* Registers the undo command
* @param cmdClass The class to be executed on undo
*/
public function registerUndoCommand( cmdClass:Class ):void
{
undoCmdClass = cmdClass;
}

b.   executeCommand – method that needs to be overridden in the sub class

:
/**
* This method must be overridden in the super class.
* Post here the code the be executed by the command.
*/
public function executeCommand():void
{
throw new Error("The undoable command does not have 'executeCommand' method implemented.");
}

c.   redo – recalls executeCommand
:
public function redo():void
{
executeCommand();
}

d.   undo – creates an instance of the undoCommand (registered at point 1) and executes it

:
public function undo():void
{
if ( undoCmdClass == null )
throw new Error("Undo command not set. Could not undo. Use 'registerUndoCommand' to register an undo command");

/** The type of the notification is used as a flag,
* indicating whether to save the command into the history, or not.
* The undo command, should not be recorded into the history,
* and its notification type is set to <code>UndoableCommandEnum.NON_RECORDABLE_COMMAND</code>
**/
var oldType:String = _note.getType();
_note.setType( UndoableCommandEnum.NON_RECORDABLE_COMMAND );

try
{
var commandInstance : ICommand = new undoCmdClass();
commandInstance.execute( _note );
}
catch ( err:Error )
{
trace("Could not call undo on " + this + ". " + err.getStackTrace() );
}

_note.setType( oldType );
}

e.   execute – this method saves the command into the history and calls executeCommand method

:
/**
* Saves the command into the CommandHistoryProxy class
* ( if <code>note.getType() == UndoableCommandEnum.RECORDABLE_COMMAND</code> )
* and calls the <code>executeCommand</code> method.
*
* @param note The Notification instance
*/
override public function execute(note:INotification):void
{
_note = note;

executeCommand();

if ( note.getType() == UndoableCommandEnum.RECORDABLE_COMMAND )
{
var historyProxy:CommandHistoryProxy = facade.retrieveProxy( CommandHistoryProxy.NAME ) as CommandHistoryProxy;
historyProxy.putCommand( this );
}
}

2.   I created command classes by extending UndoableCommandBase with 2 methods:

a.   override public function execute(note:INotification) -  that calls the super.execute( note ) in order to record the command into the history, and calls registerUndoCommand in order to save the command that will be used for undo
b.   override public function executeCommand()- method that actually does what is needed inside the command

Here is a sample of how the class looks like :

:
public class AddItemCommand extends UndoableCommandBase
{
override public function execute(note:INotification):void
{
super.execute( note );
registerUndoCommand( RemoveItemCommand );
}

override public function executeCommand():void
{
var itemsProxy:ItemsProxy = facade.retrieveProxy( ItemsProxy.NAME ) as ItemsProxy;
itemsProxy.addItem( _note.getBody() as ItemVO );
}

}

If you have better ideas I’m looking forward in seeing them. Also, for people that are interested in this solution, I can share the sources of a sample application.

Dragos

PS- Cliff, thanks for sharing with us your work. I discovered pureMVC 2 weeks ago and I already started to use it with my entire team. We’re very exited of this “world”. We’re hoping to have a better scalable and maintainable product at the end, using your solution.
Logged
puremvc
Global Moderator
Hero Member
*****
Posts: 2871



View Profile WWW Email
« Reply #3 on: December 01, 2007, 10:57:47 »

Dragos,

This looks like an interesting and viable approach.

What I'd like to see is a scheme like this abstracted to a library and used in a demo. Maybe with a simple Photoshop style history panel that allows you to go backward and forward through the Command history. Demonstrating undo/redo.

As you know it's been difficult for the community to contribute in a reasonable way up until now with only forums to post to.

Recently however, CVSDude has graciously donated space to the project. Currently I'm busy setting up new repositories for ports to other languages, but I'll also be setting up project space for utilities and demos.

If you'd like to work this into an official utility and demo for the AS3 version, I'll be happy to set up project space to devote to it. It will come with its own wiki and bug tracker as well as SVN repository.

Would you like to run such a project here?

Justin, are you still around? Would you like to work on this? I think much of this started with your detailed suggestions.

-=Cliff>
« Last Edit: December 01, 2007, 11:00:24 by puremvc » Logged
justsee
Courseware Beta
Newbie
***
Posts: 9


View Profile Email
« Reply #4 on: December 03, 2007, 04:08:59 »

Hi Cliff,
 yep still around - and just read your post re: contributions. Very interested to help out when I can, and it sounds like a good idea to start here.

Just about ready for a closed-beta launch of a major app we're releasing, which uses PureMVC, WebORB, and APE. Looking to get a chance to refine things and perhaps create some puremvc demos off the back of what I've learnt so far.

Dragos, I'm looking through your example now and would be interested in seeing your example implementation if possible.

cheers,
justin.
Logged
ddragosd
Courseware Beta
Jr. Member
***
Posts: 10


View Profile Email
« Reply #5 on: December 03, 2007, 05:46:38 »

Hi Cliff,

Your idea about making a Photoshop history panel sounds interesting and I would like to put some time aside for this, to shape it into an official demo. I can prepare something in the next two weeks . Just send me SVN credentials and I’ll post the project in there.

Dragos
Logged
puremvc
Global Moderator
Hero Member
*****
Posts: 2871



View Profile WWW Email
« Reply #6 on: December 03, 2007, 06:00:02 »

Hi Dragos,

This sounds great. Have a look at this post in the new Contributor Central forum:
http://forums.puremvc.org/index.php?topic=142.0

You'll see I'm planning an orderly approach to these contributions and I've posted some info about package space, license and attribution.

I'll contact you directly with info about SVN, etc.

-=Cliff>
Logged
justsee
Courseware Beta
Newbie
***
Posts: 9


View Profile Email
« Reply #7 on: January 08, 2008, 07:54:53 »

Hi Cliff / Dragos,
 is it possible to get access to the examples repository to check out any progress on this? I have to rework the command history component of an application I'm building, and have got a few days to focus solely on command history in PureMVC, so interested in looking at Dragos' solution.

cheers,
justin.
Logged
puremvc
Global Moderator
Hero Member
*****
Posts: 2871



View Profile WWW Email
« Reply #8 on: January 08, 2008, 08:22:27 »

Hi Justin,

Check your private messages...

-=Cliff>
Logged
ddragosd
Courseware Beta
Jr. Member
***
Posts: 10


View Profile Email
« Reply #9 on: January 08, 2008, 09:00:32 »

Hi Justin

In this case I will gladly commit the library for the command history today or tomorrow. Or if faster, I could email a zip to you.
I have been into the winter holiday until yesterday and didn't have the chance to make any commit in this time.

I wish you all a Happy New Year with this ocasion  :) !

Dragos
Logged
richardleggett
Newbie
*
Posts: 5


View Profile WWW Email
« Reply #10 on: January 09, 2009, 07:59:12 »

Hi all,

Apologies for resurrecting this thread after one year now, I'm currently building a pretty huge AIR app in fact split into 5 Flex projects (1 AIR, 3 Flex and one Zinc3, all using the same codebase) and PureMVC is really enforcing some good design.

Anyway I'm just using the contributed Undo module, I've done the relevant renaming to get it working in multicore, but of course I'm experiencing an issue and I'm not keen on my workaround, and wonder if you have any better ideas?

The most common issue I've encountered in multicore of course is the use of the facade reference in INotifiers before they are registered (and therefore have the multiton key, which in my case is never accessible outside of the Facade subclass).

One example of this is in a Command class. Generally this is not a problem because the Commands are registered in your facade class and automatically given the multiton key before you use them. But... in the case of undo this appears to break down a little because the undo command is never given the multiton key.

The simple fix is to modify UndoableCommandBase to include one line (line 2 of these 3 lines):

:
var commandInstance : ICommand = new undoCmdClass();
commandInstance.initializeNotifier(multitonKey);
commandInstance.execute( _note );

Would that be a reasonable thing to do or is this bad practice because I'm re-using the multiton key directly?

Thanks and glad to have found this extension.
Logged
puremvc
Global Moderator
Hero Member
*****
Posts: 2871



View Profile WWW Email
« Reply #11 on: January 10, 2009, 08:22:35 »

This fix looks correct.

The Undo utility has been in a development state for quite a while and I haven't heard from Dracos, the author, about its definitive status. If you don't mind, send the author an email from the link on his contributor page, and see what he thinks. If you don't mind, also post your findings with the utility here.

Thanks,
-=Cliff>
Logged
ddragosd
Courseware Beta
Jr. Member
***
Posts: 10


View Profile Email
« Reply #12 on: January 12, 2009, 10:05:55 »

The current release is stable and ready to use. I didn't use it in multicore version of the framework, and indeed the fix is perfect. I'm glad it helped you Richard.
Logged
richardleggett
Newbie
*
Posts: 5


View Profile WWW Email
« Reply #13 on: January 12, 2009, 10:59:16 »

Thank guys, I've got a long way to go before I can really report anything interesting, but the Undo is working as it should, any user or programmable actions such as selecting an item on screen, moving/scaling an item and so on are turned into a "action Command" (mostly this is a SetPropertyCommand or SelectItemCommand) and a params object which feeds it the right data and also whether the command is to be recorded in the history stack.

This "action command" just calls methods on the proxies to do its work as per normal PureMVC. In order to actually instantiate and execute this there's actually a PerformActionCommand which creates an instance of the action command class and feeds it the relevant data from the notification body. It's all quite generesized so it's quite hard to describe well. This is the bit I'm not too happy on as I'm having to dynamically construct a Command instance myself and initialize it with the multiton key. I can't really think up a better way of chaining up this command sequence ahead of runtime.

When it comes to the undo... upon sending a CommandHistoryProxy.UNDO_LAST notification (a notification I added) the controller is executing a generic UndoLastCommand command (the naming should be UndoLastCommandCommand to be honest but that looks ugly). This simply checks CommandHistoryProxy.canUndo and then calls getPrevious() to call pop the last IUndoableCommand off the stack in order to finally call undo() on that, again in my case this is usually an UndoSetPropertyCommand or UndoSelectItemCommand.
Logged
ddragosd
Courseware Beta
Jr. Member
***
Posts: 10


View Profile Email
« Reply #14 on: January 12, 2009, 11:33:45 »

Richard, I'm not sure if I got it right, but I'm wondering why would you need an UndoSetPropertyCommand, if you already have SetPropertyCommand. Can't you undo SetPropertyCommand with SetPropertyCommand ?

In my projects I do something like this:
1. I have an UpdateCommand that undos itself, so the registered undo command is also UpdateCommand
2. When I call that command I send the updated state of the object I want to change into the body of the note. I.e. note.getBody() would be an instance of ItemVO, let's call it 'newProperties'
3. When I execute the command, I save a snapshot with the object I am about to change, so I retrieve a copy of ItemVO, let's call it 'originalProperties'
3. I perform the changes in the model so that the view updates properly according to the 'newProperties'
4. I copy the snapshot 'originalProperties' into the note's body because this is the information I need to call undo.
5. Calling undo/redo it will simply switch 'newProperties' with 'originalProperties'.

I don't know if this can be applied to your case though.
Logged
Pages: [1] 2
Print