[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: [PATCH] RunnerManager
From: "Jordi Polo" <mumismo () gmail ! com>
Date: 2008-04-30 4:14:37
Message-ID: a4162420804292114pdc8105foe539d09d4f26ba3c () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> ignoredType is wrong. it should be:
>
> SearchContext::Types ignoredTypes() const
>
> using the flags (e.g. ignoredTypes & type) would done on the consumer's
> end
> (e.g. in RunnerManager)
>
Done
>
> the Private copy ctor shouldn't copy the matches (it used only on detach,
> right? so we don't want the matches). this makes the call to
> removeAllMatches
> unecessary in reset(). that's the whole point of this approach: avoid
> copying, deleting, etc.
>
Ok, the ctor should initialize all to empty objects as it is only called in
reset. I'll change that. See my next commentary.
> + if (this !=d->q) {
> + //FIXME: this line must be uncommented.
> + d.detach();
> + }
>
> that looks completely backwards to me. it also looks completely
> unecessary.
> whenever reset is called it should detach. in practice, reset will only be
> called on the real context object, and that one should detach... again,
> this
> all about preventing copies and deletions.
Detaching always is ok. Detaching will create a new empty object, so we
don't need the rest of the method when we have indeed detached (we still
need to clean our state if we don't detach).
But detach() will only effectively detach if other object references the
data, as detach() returns void, we can not know if in fact detached. I am
tempted to use a bool to indicate if we are in a clean state, but it looks
terrible ugly.
>
>
> the rest looks ok ... work needed in places still, but close enough ...
> i'm
> tempted to say "commit it, and then i'll polish it up"
>
Puff, at last we got close, after the detach issue is solved I'll comment
kDebug() and commit.
--
Jordi Polo Carres
NLP laboratory - NAIST
http://www.bahasara.org
[Attachment #5 (text/html)]
<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px \
solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">ignoredType \
is wrong. it should be:<br> <br>
SearchContext::Types ignoredTypes() const<br>
<br>
using the flags (e.g. ignoredTypes & type) would done on the consumer's \
end<br> (e.g. in RunnerManager)<br>
</blockquote><div><br>Done<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> the Private copy ctor shouldn't copy the matches (it \
used only on detach,<br> right? so we don't want the matches). this makes the \
call to removeAllMatches<br> unecessary in reset(). that's the whole point of \
this approach: avoid<br> copying, deleting, etc.<br>
</blockquote><div><br>Ok, the ctor should initialize all to empty objects as it is \
only called in reset. I'll change that. See my next \
commentary.<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>
+ if (this !=d->q) {<br>
+ //FIXME: this line must be uncommented.<br>
+ d.detach();<br>
+ }<br>
<br>
that looks completely backwards to me. it also looks completely unecessary.<br>
whenever reset is called it should detach. in practice, reset will only be<br>
called on the real context object, and that one should detach... again, this<br>
all about preventing copies and deletions.</blockquote><div><br>Detaching always is \
ok. Detaching will create a new empty object, so we don't need the rest of the \
method when we have indeed detached (we still need to clean our state if we don't \
detach). <br> But detach() will only effectively detach if other object references \
the data, as detach() returns void, we can not know if in fact detached. \
I am tempted to use a bool to indicate if we are in a clean state, but it looks \
terrible ugly. <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> <br>
the rest looks ok ... work needed in places still, but close enough ... i'm<br>
tempted to say "commit it, and then i'll polish it up"<br>
<font color="#888888"></font></blockquote><div><br>Puff, at last we got close, after \
the detach issue is solved I'll comment kDebug() and \
commit.<br><br></div></div><br><br clear="all"><br>-- <br>Jordi Polo Carres<br> NLP \
laboratory - NAIST<br><a \
href="http://www.bahasara.org">http://www.bahasara.org</a><br>
_______________________________________________
Panel-devel mailing list
Panel-devel@kde.org
https://mail.kde.org/mailman/listinfo/panel-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic