[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 15:51:08
Message-ID: ae32c1ef0909290851i367dc02cta331baf5e6072b30 () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Tue, Sep 29, 2009 at 3:09 PM, Thomas Zander <zander@kde.org> wrote:
> On Tuesday 29. September 2009 13.56.40 Dmitry Kazakov wrote:
> > On Tue, Sep 29, 2009 at 2:28 PM, Thomas Zander <zander@kde.org> wrote:
> > > On Tuesday 29. September 2009 13.15.54 Dmitry Kazakov wrote:
> > > > 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?
> > >
> > > The code is;
> > >
> > > m_doc->undoStack()->undo();
> > >
> > > undoStack is a public method, not a private member. There is no
> breaking
> > > of encapsulation in this code.
> >
> > There is. The object, returned by undoStack(), *is* private.
>
> As soon as the public assessor was added, it stopped being private
It's just a workaround that hides broken logic of the object.
and thus
> all results of it becoming public have to be taken. Good and bad.
>
> The good thing is that you can use it and don't have to create new methods,
>
Are you that lazy to create a couple of methods? And allow "spaghetti code"
instead?
the bad is that KoDocuments public API states we use the QUndoStack, so we
> can't stop using that.
>
Why not just derive KoDocument form QUndoStack? Like
class KoDocument : QUndoStack {
...
};
It would make an api clearer and more maintainable. Example, if you decide
to change undo() method one day (e.g. add locking), you will just need to
overload one virtual method instead of overloading the whole QUndoStack and
changing all appropriate constructors in KoDocument.
>
> I think its just fine to decide in our API that we use and will continue to
> use
> the QUndoStack (or inherited). So I don't share your concerns and as we are
> indeed not breaking encapsulation due to that prior decision,
Please take a look at this:
http://en.wikipedia.org/wiki/Object-oriented_programming#Fundamental_concepts_and_features
"Method - an object's abilities. In language, methods (sometimes referred to
as "functions") are verbs. Lassie, being a Dog, has the ability to bark."
In our case, KoDocument do not have any ability "to UndoStack". It is not a
verb even.
More than that, usually, return values of the functions are considered as
"out-values". But the construction above uses them as an 'in-value'.
> its fine to use
> the code snipped pasted above.
>
Well, zooming subsystem has already become unusable because of the access to
private members (managers have cyclic links to each other). That's why it
has hacks now. Do you really want the same fate for KoDocument? "API design
matters" [0]
Btw, if you still insist on your method, please answer a question: how are
you going to add locking for undo() operations in the future? For example,
one day you will need to acquire "BigSomeStuffLock" on every undo. How are
you going to do this?
[0] - http://mags.acm.org/communications/200905/?pg=48&pm=2
--
Dmitry Kazakov
[Attachment #5 (text/html)]
<br><br><div class="gmail_quote">On Tue, Sep 29, 2009 at 3:09 PM, Thomas Zander <span \
dir="ltr"><<a href="mailto:zander@kde.org">zander@kde.org</a>></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;"> <div class="im">On Tuesday 29. \
September 2009 13.56.40 Dmitry Kazakov wrote:<br> > On Tue, Sep 29, 2009 at 2:28 \
PM, Thomas Zander <<a href="mailto:zander@kde.org">zander@kde.org</a>> \
wrote:<br> > > On Tuesday 29. September 2009 13.15.54 Dmitry Kazakov wrote:<br>
> > > Argh! =(<br>
> > > You are beaking encapsulation! Isn't this a basic principle of<br>
> > > object-oriented programming?<br>
> > > There is an object (doc), and there is it's interface<br>
> > > (undoLastCommand). And no calls to private members! (undo stack is<br>
> > > private, right?) Why do<br>
> ><br>
> > you<br>
> ><br>
> > > use private members?<br>
> ><br>
> > The code is;<br>
> ><br>
> > m_doc->undoStack()->undo();<br>
> ><br>
> > undoStack is a public method, not a private member. There is no \
breaking<br> > > of encapsulation in this code.<br>
><br>
> There is. The object, returned by undoStack(), *is* private.<br>
<br>
</div>As soon as the public assessor was added, it stopped being private \
</blockquote><div><br>It's just a workaround that hides broken logic of the \
object.<br> <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;"> and \
thus<br> all results of it becoming public have to be taken. Good and \
bad.<br></blockquote><blockquote class="gmail_quote" style="border-left: 1px solid \
rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> <br>
The good thing is that you can use it and don't have to create new \
methods,<br></blockquote><div><br>Are you that lazy to create a couple of methods? \
And allow "spaghetti code" instead?<br> <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;">
the bad is that KoDocuments public API states we use the QUndoStack, so we<br>
can't stop using that.<br></blockquote><div><br>Why not just derive KoDocument \
form QUndoStack? Like<br><br>class KoDocument : QUndoStack {<br>...<br>};<br><br>It \
would make an api clearer and more maintainable. Example, if you decide to change \
undo() method one day (e.g. add locking), you will just need to overload one virtual \
method instead of overloading the whole QUndoStack and changing all appropriate \
constructors in KoDocument.<br> <br><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;"> <br>
I think its just fine to decide in our API that we use and will continue to use<br>
the QUndoStack (or inherited). So I don't share your concerns and as we are<br>
indeed not breaking encapsulation due to that prior \
decision,</blockquote><div><br>Please take a look at this:<br><a \
href="http://en.wikipedia.org/wiki/Object-oriented_programming#Fundamental_concepts_an \
d_features">http://en.wikipedia.org/wiki/Object-oriented_programming#Fundamental_concepts_and_features</a><br>
<br>"Method - an object's abilities. In language, methods (sometimes \
referred to as "functions") are verbs. <code>Lassie</code>, being a \
<code>Dog</code>, has the ability to bark."<br><br>In our case, KoDocument do \
not have any ability "to UndoStack". It is not a verb even.<br> <br>More \
than that, usually, return values of the functions are considered as \
"out-values". But the construction above uses them as an \
'in-value'.<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;"> its fine to use<br>
the code snipped pasted above.<br></blockquote></div><br>Well, zooming subsystem has \
already become unusable because of the access to private members (managers have \
cyclic links to each other). That's why it has hacks now. Do you really want the \
same fate for KoDocument? "API design matters" [0]<br> <br><br>Btw, if you \
still insist on your method, please answer a question: how are you going to add \
locking for undo() operations in the future? For example, one day you will need to \
acquire "BigSomeStuffLock" on every undo. How are you going to do this?<br> \
<br><br clear="all">[0] - <a \
href="http://mags.acm.org/communications/200905/?pg=48&pm=2">http://mags.acm.org/communications/200905/?pg=48&pm=2</a><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]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic