[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    Re: koffice
From:       Dmitry Kazakov <dimula73 () gmail ! com>
Date:       2009-09-29 11:15:54
Message-ID: ae32c1ef0909290415h6de2da04j11ea21f368b0708f () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Tue, Sep 29, 2009 at 11:51 AM, Thomas Zander <zander@kde.org> wrote:

> SVN commit 1029197 by zander:
>
> Inline the KoDocument::undoLastCommand, no need for a virtual method.
>
>  M  +1 -1      krita/image/kis_undo_adapter.cc
>  M  +0 -5      libs/main/KoDocument.cpp
>  M  +0 -5      libs/main/KoDocument.h
>
>
> --- trunk/koffice/krita/image/kis_undo_adapter.cc #1029196:1029197
> @@ -78,7 +78,7 @@
>      * command->undo() and remove the cache without committing it
>      * to the undoStack
>      */
> -    m_doc->undoLastCommand();
> +    m_doc->undoStack()->undo();
>  }
>

Argh! =(
You are beaking encapsulation! Isn't this a basic principle of
object-oriented programming?
There is an object (doc), and there is it's interface (undoLastCommand). And
no calls to private members! (undo stack is private, right?) Why do you use
private members?

These calls to privates are curse of our code... They make it simply
unmaintainable... Just remember zooming subsys and PixelAspect button
case...

I'm insisting on reverting this. I admit that this function may be
'non-virtual', i simply didn't manage to compile this as non-virtual. =(





>  void KisUndoAdapter::setUndo(bool undo)
> --- trunk/koffice/libs/main/KoDocument.cpp #1029196:1029197
> @@ -2381,11 +2381,6 @@
>         d->undoStack->push(command);
>  }
>
> -void KoDocument::undoLastCommand()
> -{
> -    d->undoStack->undo();
> -}
> -
>  void KoDocument::beginMacro(const QString & text)
>  {
>     d->undoStack->beginMacro(text);
> --- trunk/koffice/libs/main/KoDocument.h #1029196:1029197
> @@ -790,11 +790,6 @@
>     virtual void addCommand(QUndoCommand* command);
>
>     /**
> -     * Undo the command that is stored at the top of the stack
> -     */
> -    virtual void undoLastCommand();
> -
> -    /**
>      * Begins recording of a macro command. At the end endMacro needs to be
> called.
>      * @param text command description
>      */
>



-- 
Dmitry Kazakov

[Attachment #5 (text/html)]

<br><br><div class="gmail_quote">On Tue, Sep 29, 2009 at 11:51 AM, Thomas Zander \
<span dir="ltr">&lt;<a href="mailto:zander@kde.org" \
target="_blank">zander@kde.org</a>&gt;</span> wrote:<br><blockquote \
class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt \
0pt 0.8ex; padding-left: 1ex;">

SVN commit 1029197 by zander:<br>
<br>
Inline the KoDocument::undoLastCommand, no need for a virtual method.<br>
<br>
  M   +1 -1         krita/image/kis_undo_adapter.cc<br>
  M   +0 -5         libs/main/KoDocument.cpp<br>
  M   +0 -5         libs/main/KoDocument.h<br>
<br>
<br>
--- trunk/koffice/krita/image/kis_undo_adapter.cc #1029196:1029197<br>
@@ -78,7 +78,7 @@<br>
         * command-&gt;undo() and remove the cache without committing it<br>
         * to the undoStack<br>
         */<br>
-      m_doc-&gt;undoLastCommand();<br>
+      m_doc-&gt;undoStack()-&gt;undo();<br>
  }<br></blockquote><div><br>Argh! =(<br>You are beaking encapsulation! Isn&#39;t \
this a basic principle of object-oriented programming?<br>There is an object (doc), \
and there is it&#39;s interface (undoLastCommand). And no calls to private members! \
(undo stack is private, right?) Why do you use private members?<br> <br>These calls \
to privates are curse of our code... They make it simply unmaintainable... Just \
remember zooming subsys and PixelAspect button case...<br><br>I&#39;m insisting on \
reverting this. I admit that this function may be &#39;non-virtual&#39;, i simply \
didn&#39;t manage to compile this as non-virtual. =(<br>  <br></div><div><br><br>  \
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, \
204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">  void \
                KisUndoAdapter::setUndo(bool undo)<br>
--- trunk/koffice/libs/main/KoDocument.cpp #1029196:1029197<br>
@@ -2381,11 +2381,6 @@<br>
             d-&gt;undoStack-&gt;push(command);<br>
  }<br>
<br>
-void KoDocument::undoLastCommand()<br>
-{<br>
-      d-&gt;undoStack-&gt;undo();<br>
-}<br>
-<br>
  void KoDocument::beginMacro(const QString &amp; text)<br>
  {<br>
       d-&gt;undoStack-&gt;beginMacro(text);<br>
--- trunk/koffice/libs/main/KoDocument.h #1029196:1029197<br>
@@ -790,11 +790,6 @@<br>
       virtual void addCommand(QUndoCommand* command);<br>
<br>
       /**<br>
-       * Undo the command that is stored at the top of the stack<br>
-       */<br>
-      virtual void undoLastCommand();<br>
-<br>
-      /**<br>
         * Begins recording of a macro command. At the end endMacro needs to be \
                called.<br>
         * @param text command description<br>
         */<br>
</blockquote></div><br><br clear="all"><br>-- <br>Dmitry Kazakov<br>



_______________________________________________
kimageshop mailing list
kimageshop@kde.org
https://mail.kde.org/mailman/listinfo/kimageshop


[prev in list] [next in list] [prev in thread] [next in thread]