Who know the best way to make an UNDO step for graphical application

Who know the best way to make an UNDO step for graphical application

• cnceric

  The best way to implement undo/redo is to implement the Command pattern. Here is a link that will show you how to implement it. http://www.codeproject.com/cs/design/sharped.asp.
  
  I have also used this pattern and the Memento pattern, it is simpler but you need to consider the cost of saving the state for each action.The memento pattern takes the state of the object before the action and saves it, to undo you just replace the state with the previous one. The way I implemented it was to have 2 ArrayLists, one to hold the previous state and one to hold redo states ( move state into redo when you do a undo).
  
  Here is an example from my code, the CopyState function takes a deep copy of the object,LandmarkSetState is a relatively dumb class that is used mainly for holding data.
  public void SetState(LandmarkSetState state)
  {
  if(m_currentState != null)
  m_previousStates.Add(m_currentState.CopyState());
  m_currentState = state.CopyState();
  m_undoneStates.Clear();
  m_undoneStates.TrimToSize();
  }
  public LandmarkSetState Undo()
  {
  if (m_previousStates.Count > 0)
  {
  m_undoneStates.Add(m_currentState);
  m_currentState = (LandmarkSetState)m_previousStates[m_previousStates.Count-1]; m_previousStates.RemoveAt(m_previousStates.Count-1);
  }
  return m_currentState.CopyState();
  }
  public LandmarkSetState Redo()
  {
  if (m_undoneStates.Count > 0)
  {
  m_previousStates.Add(m_currentState);
  m_currentState = (LandmarkSetState)m_undoneStates[m_undoneStates.Count-1]; m_undoneStates.RemoveAt(m_undoneStates.Count-1);
  }
  return m_currentState.CopyState();
  }
  

• JoeWood

  I'm not sure if you are confusing the term "decouple". The problem that you are describing I would say is that you are doing a shallow copy (referencing the original object), therefore any changes you make are also made by the copy saved for the undo.
  If this is the problem you need to do a deep copy, which means creating new instance of reference types, ensuring that all data in the object and all objects that it contains. Here is how I did it. Notice that the reference types are copied using "new" to create a completely new copy, value types okay.
  
  virtual public LandmarkSetState CopyState()
  {
  LandmarkSetState temp = new LandmarkSetState();
  temp.m_landmarks = new ArrayList();
  temp.m_id = m_id;
  for (int i =0; i < m_landmarks.Count; i++)
  {
  Landmark lm = new Landmark((Landmark)m_landmarks); // creates a completely new instance of the object.
  temp.m_landmarks.Add(lm);
  }
  temp.m_selectedLandmark = m_selectedLandmark;
  temp.m_curveColour = m_curveColour;
  temp.m_showNumbers = m_showNumbers;
  temp.m_area = m_area;
  temp.m_lastToFirst = m_lastToFirst;
  temp.m_connections = new ArrayList(m_connections);
  temp.m_ShowConnections = m_ShowConnections;
  temp.m_maxNumberOfLandmarks = m_maxNumberOfLandmarks;
  temp.m_allSelected = m_allSelected;
  temp.m_splineTension = m_splineTension;
  temp.m_workingState = m_workingState;
  temp.m_mouseState = m_mouseState;
  temp.m_mouseMoved = m_mouseMoved;
  temp.m_zoomFactor = m_zoomFactor;
  
  return temp;
  }
  
  
  

• Craig G

  I think you should consider GoF Memento Design Pattern
  provided exmaple suits in your case
  
  hope this helps
  
  
  
  

• Chuff

  Hi Alan

  Thank again for your commend , I did a deep copy as you did , I did that by invoking a function CopyInstant , this code written in Visual Basic ,its coding style is similar to C# that you use here ( C# is some thing in between C++ and Visual Basic)

  The bellow in function core

  Public Function CopyInstant(ByVal CopyObject As GrapDesignBase) As GrapDesignBase

  Dim ObjectType As String

  Dim ReturnObject As GrapDesignBase

  ObjectType = CopyObject.GetType.Name

  With CopyObject

  Select Case ObjectType

  Case "LineObject"

  Dim LObj As New LineObject(.X, .Y)

  Me.CopyProperty(CopyObject, LObj)

  ReturnObject = CType(LObj, GrapDesignBase)

  Case "RectangleObject"

  Dim RectObj As New RectangleObject(.X, .Y)

  Me.CopyProperty(CopyObject, RectObj)

  ReturnObject = CType(RectObj, GrapDesignBase)

  Case "EllipseObject"

  Dim ElObj As New EllipseObject(.X, .Y)

  Me.CopyProperty(CopyObject, ElObj)

  ReturnObject = CType(ElObj, GrapDesignBase)

  Case "TextObject"

  Dim T As TextObject

  Dim TextObj As New TextObject

  T = CType(CopyObject, TextObject)

  If T.FontType = TextObject.FontEnum.EngravedFont Then

  TextObj = New TextObject(T.X, T.Y, T.Text, T.EngravedFont, T.LineColor, T.Width, T.Height)

  End If

  If T.FontType = TextObject.FontEnum.TrueTypeFont Then

  TextObj = New TextObject(T.X, T.Y, T.Text, T.Font, T.LineColor, T.Width, T.Height)

  End If

  ReturnObject = CType(TextObj, GrapDesignBase)

  Case "EmbeddedImageObject"

  Dim Im As EmbeddedImageObject

  Im = CType(CopyObject, EmbeddedImageObject)

  Dim EmIm As New EmbeddedImageObject(Im.X, Im.Y, Im.Image)

  ReturnObject = CType(EmIm, GrapDesignBase)

  Case "LinkedImage"

  Dim Im As LinkedImage

  Im = CType(CopyObject, LinkedImage)

  Dim LinIm As New LinkedImage(Im.X, Im.Y, Im.ImagePath)

  ReturnObject = CType(LinIm, GrapDesignBase)

  Case "Polygon"

  Dim P As Polygon

  P = CType(CopyObject, Polygon)

  Dim Pol As New Polygon(P.center, P.R, P.NumEdge)

  Me.CopyProperty(CopyObject, Pol)

  ReturnObject = CType(Pol, GrapDesignBase)

  Case "PolyLine"

  Dim Pol As PolyLine

  Pol = CType(CopyObject, PolyLine)

  Dim Polline As New PolyLine(Pol.MB_MyPath)

  Me.CopyProperty(CopyObject, Polline)

  ReturnObject = CType(Polline, GrapDesignBase)

  Case "CircularText"

  Dim CirText As CircularText

  Dim NewObject As New CircularText()

  CirText = CType(CopyObject, CircularText)

  With CirText

  If .FontType = TextObject.FontEnum.EngravedFont Then

  NewObject = New CircularText(New Point(.MB_Center1.X, .MB_Center1.Y), .Text, .EngravedFont, .MB_TextSize, .Radius, .StartAngle, .SweepAngle)

  End If

  If .FontType = TextObject.FontEnum.TrueTypeFont Then

  NewObject = New CircularText(New Point(.MB_Center1.X, .MB_Center1.Y), .Text, .Font, .Radius, .StartAngle, .SweepAngle)

  End If

  NewObject.Rotation = CirText.Rotation

  NewObject.SelectPenColor = CirText.SelectPenColor

  ReturnObject = CType(CirText, GrapDesignBase)

  End With

  Case Else

  ReturnObject = Nothing

  End Select

  End With

  Return ReturnObject

  End Function

  So this function will deep copy an original object which is a parater that passing to it .

  So you will see that I did a deep copy eventhough I still encounter this problem.

  
  

• Roni Schuetz

  Sorry for not getting back sooner, I was on hols.
  Are you sure the problem is where you think it is; you appear to be doing things correctly. To test it out I would do the following:
  

  1. put a breakpoint at the line of code that does the copy.
  2. step over the line so that you should now have objects.
  3. using the watch window change the values in one of the objects and see if they change in the other - if they change you have a problem with you deep copy otherwise it is somewhere else in your code.

  If you like you can send me a zipped up version of the application and I'll have a look at it.
  
  Regards
  Alan
  
  
  

• AhXue

  Thank you Alan for your comment, it is my fault that I didn't give you a metod body of CopyProperty so that it misleading you.

  Actually what you are suggested is done inside a Sub Name CopyProperty . In visual Basic Sub is a function that return Void in C++ and C# , it take parameter in this case and in some other case it also may take Void type parameter, bellow is its body

  Public Sub CopyProperty(ByVal SourceObject As GrapDesigns.ShapeObject, ByVal DestObject As GrapDesigns.ShapeObject)

  DestObject.LineWidth = SourceObject.LineWidth

  DestObject.LineColor = SourceObject.LineColor

  DestObject.Rotation = SourceObject.Rotation

  DestObject.FillColor = SourceObject.FillColor

  DestObject.Fill = SourceObject.Fill

  DestObject.Width = SourceObject.Width

  DestObject.Height = SourceObject.Height

  DestObject.WidthF = SourceObject.WidthF

  DestObject.HeightF = SourceObject.HeightF

  DestObject.HatchFill = SourceObject.HatchFill

  DestObject.HatchStyle = SourceObject.HatchStyle

  DestObject.SelectPenColor = SourceObject.SelectPenColor

  End Sub

  Actually if you go back and look at my previous code you will see that After I instantiate a new object with its constructor I just copy all property of the source object here is RectObj to a newly created object which is CopyObject , in the body of Sub CopyProperty is doing this procedure which is equivalent to what you suggested. The same scenario I did for Rectangular and Circular Array ,but these two function work perfectly . I may need to check where it is wrong in my code ( some whereelse but not here)

  
  

• Mike Wachal - MSFT

  I've never programmer in VB .NET but looking at the code it looks like you are doing a shallow copy still. This what I think is wrong. I've given a simple example of what I think should work. My VB is poor so ignore syntax errors.
  
  

  Case "RectangleObject"

  Dim RectObj As New RectangleObject(.X, .Y) ///1. you create a new object values in the constructor
  

  Me.CopyProperty(CopyObject, RectObj) ///2. you copy the copy object into the new object (shallow copy)
  

  ReturnObject = CType(RectObj, GrapDesignBase) ///3. you assign the RectObj to the return object
  

  
  Is it not possible to do something like this
  
  1. Create the return object
  

  Dim ReturnObject As GrapDesignBase

  
  2. Assign the values to the return object
  
  ReturnObject.Name = new String(CopyObject.Name) // assuming a reference type
  ReturnObject.Age = CopyObject.Age // assuming a value type
  
  3. Return the object
  
  Return ReturnObject
  
  

• Dreedle

  Thank you Galin

  I went to the link ,but there is quite abstract, there is no concrete implementation sample some time an abstract desciption is OK but on the implementation there are a lot of problems . Abstract is like a theory ,in this case I need more implementation sample, I understood the idea but to realize this idea in practice

  
  

• needforhint

  Thank you Alan

  for your recommendation, the thing I was done is similar to what you are recommended but I couldn't decouple the original object and its copy as I mentioned in the question . Go through the link you provided , I think I need to create a separate class to handle this with this thougth my class's implement code need to rewrite ( I made all drawing associate classes as a DLL but the actual implementation code import this DLL ,so all action like move ,rotate,size are in the implement code) . But one thing that I worry ( base on the code shown in the link and your code) how we can ensure that the previous state keep unchange when the original object change or in another word how we can ensure that they are decouple. My code by concept did almost the same thing as you did and the code on the link did ,if the previous state keep changing according to the original object does , after undo we will get a new state instead of previous state.

  Thus means the undo action does'nt work, my key problem is how to decouple the original object from its copy.

  Regards

  
  
• Who know the best way to make an UNDO step for graphical application