<p><br>
27.07.2011 3:12 пользователь "Christian Mollekopf" <<a href="mailto:chrigi_1@fastmail.fm">chrigi_1@fastmail.fm</a>> написал:<br>
><br>
> On Tue, 26 Jul 2011 20:57:46 +0200, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a>> wrote:<br>
><br>
> > On 07/26/2011 05:06 PM, Christian Mollekopf wrote:<br>
> >> On Tue, 26 Jul 2011 13:26:20 +0200, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
> >> wrote:<br>
> >><br>
> >>> On 07/25/2011 09:43 PM, Christian Mollekopf wrote:<br>
> >>>> On Mon, 25 Jul 2011 21:20:45 +0200, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
> >>>> wrote:<br>
> >>>><br>
> >>>>> On 07/25/2011 07:52 PM, Christian Mollekopf wrote:<br>
> >>>>>> On Mon, 25 Jul 2011 18:30:05 +0200, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
> >>>>>> wrote:<br>
> >>>>>><br>
> >>>>>>> Now that the SimpleResource rcgen is in git master[1] some people<br>
> >>>>>>> used<br>
> >>>>>>> it (mostly for Akonadi feeders and the web extractor framework) and<br>
> >>>>>>> found it to be a bit strange.<br>
> >>>>>>><br>
> >>>>>>> This is true. Current usage is as follows:<br>
> >>>>>>><br>
> >>>>>>> SimpleResource r;<br>
> >>>>>>> NCO::PersonContact(&r).setFullname("foobar");<br>
> >>>>>>><br>
> >>>>>>> The reason for this is simple: due to multi-inheritance problems I<br>
> >>>>>>> cannot derive the generated classes from SimpleResource. Thus, we<br>
> >>>>>>> need<br>
> >>>>>>> wrappers.<br>
> >>>>>>><br>
> >>>>>>> Now some ideas were floating around and I would like to settle on<br>
> >>>>>>> one<br>
> >>>>>>> better solution.<br>
> >>>>>>><br>
> >>>>>>> One idea would be to have a hybrid thing where you could do this:<br>
> >>>>>>><br>
> >>>>>>> NCO::PersonContact pc;<br>
> >>>>>>> pc.setFullname("foobar");<br>
> >>>>>>> SimpleResource r = rc.resource();<br>
> >>>>>>><br>
> >>>>>>> and this:<br>
> >>>>>>><br>
> >>>>>>> SimpleResource r;<br>
> >>>>>>> NCO::PersonContact pc(r);<br>
> >>>>>>> pc.setFullname("foobar");<br>
> >>>>>>> r = pc.resource();<br>
> >>>>>>><br>
> >>>>>>> (The last line would be required since all SimpleResources would be<br>
> >>>>>>> copies.)<br>
> >>>>>><br>
> >>>>>> Why can't you just take the reference to r, and the last copy<br>
> >>>>>> wouldn't<br>
> >>>>>> be<br>
> >>>>>> needed?<br>
> >>>>>> Of course the accessor is still needed for the NCO::PersonContact<br>
> >>>>>> pc;<br>
> >>>>>> usecase.<br>
> >>>>><br>
> >>>>> I cannot use refs for security reasons. Imagine someone returns a<br>
> >>>>> generated class from a method after putting in a ref to a local<br>
> >>>>> SimpleRes. Anyway, SimpleRes is implicitly shared.<br>
> >>>><br>
> >>>> Thats the same as with pointers, and it's c++ after all, developers<br>
> >>>> have<br>
> >>>> to think a bit.<br>
> >>>> I just think that forgetting r = pc.resource() is more likely than<br>
> >>>> returning a generated class<br>
> >>>> containing a local reference, because that is a problem you should be<br>
> >>>> used<br>
> >>>> to as c++ developer<br>
> >>>> (although a bit nasty I admit) and<br>
> >>>><br>
> >>>> SimpleResource r;<br>
> >>>> NCO::PersonContact pc(r);<br>
> >>>> pc.setFullname("foobar");<br>
> >>>><br>
> >>>> doesn't really look "wrong" at all (unless you have a close look at<br>
> >>>> the<br>
> >>>> header).<br>
> >>><br>
> >>> Actually I think this only looks quite correct to you since you have<br>
> >>> been using the old rcgen. If you look at pretty much all other Qt/KDE<br>
> >>> APIs you will see that this is a typical copy situation.<br>
> >>><br>
> >><br>
> >> Except if you think the class is a wrapper.<br>
> >> The copying seems to me more java style.<br>
> ><br>
> > Well, the problem is that we want wrapping and a member SimpleResource...<br>
> ><br>
> >>> And I would really prefer not to use references. Aside from the<br>
> >>> unwritten policy in Qt and KDE code to not do it it is so very error<br>
> >>> prone since people tend to overlook the little "&". That is also why I<br>
> >>> chose to use a pointer in the original version. It forces one to add<br>
> >>> the<br>
> >>> "&" ones self and, thus, think about what one it doing.<br>
> >>><br>
> >><br>
> >> Ok, my intuition is different, also before using the old rcgen.<br>
> >> Also I happen to like references =)<br>
> ><br>
> > They are very powerful indeed.<br>
> ><br>
> >> Further, just disable the copy constructor in the wrapper classes and<br>
> >> all<br>
> >> problems are gone (I reckon).<br>
> ><br>
> > We could do that but that would not solve the problem with the<br>
> > reference. We want to allow both<br>
> > SimpleResource res;<br>
> > Contact c(res);<br>
> > and<br>
> > Contact c;<br>
> > c.resource();<br>
> > That means that we have to store a pointer and convert the reference to<br>
> > a pointer (ugly!) since a reference member needs to be initialized in<br>
> > the constructor. Thus, the second case would mean to create a<br>
> > SimpleResource on the heap which leads to memory management trouble.<br>
What if instead of storing SimpleResource (or ref or pointer) store ptr to SimpleResource::Private( and make it less private, of course ) ? This way Contact c(r); will share with r it's privates, Contact c; will create new private and in all cases c.resource(); will detach a private and create a new SimpleResource with detached private copy?<br>
><br>
> Ok, you're right, I completely forgot about the second usecase =)<br>
><br>
> ><br>
> > Cheers,<br>
> > Sebastian<br>
> ><br>
> >>>> Anyways, no strong preference from my side.<br>
> >>><br>
> >>> So not get me wrong: I like the idea of making it fast. But I am also<br>
> >>> very concerned about usability of the API. I would rather have it be<br>
> >>> pretty than super-fast. And in the end the overhead of copying a few<br>
> >>> pointers or primitives is nothing compared to the actual calling of<br>
> >>> storeResources. ;)<br>
> >><br>
> >> Agreed, it's not really a performance issue.<br>
> >> I just don't like that last "r = pc.resource();", which is imo<br>
> >> unnecessary, error prone and<br>
> >> feels to me like java (where you never modify the object itself but<br>
> >> always<br>
> >> get a copy back).<br>
> >><br>
> >> Anyways, it's really just my personal opinion/preference, and I'm fine<br>
> >> with either way.<br>
> >> Especially since you did a good job in providing a nice api so far =)<br>
> >><br>
> >> Cheers,<br>
> >> Christian<br>
> >><br>
> >>><br>
> >>> Cheers,<br>
> >>> Sebastian<br>
> >>><br>
> >>>> Cheers<br>
> >>>><br>
> >>>>><br>
> >>>>>>><br>
> >>>>>>> We could go on like so:<br>
> >>>>>>><br>
> >>>>>>> r.addProperty(...);<br>
> >>>>>>> pc = r;<br>
> >>>>>>> pc.setResource(r);<br>
> >>>>>>><br>
> >>>>>>> The only disadvantage I see here might be a slight performance<br>
> >>>>>>> overhead<br>
> >>>>>>> due to the copying of the resources.<br>
> >>>>>>><br>
> >>>>>>> Opinions?<br>
> >>>>>><br>
> >>>>>> Additionally it might be nice to have an accessor to the uri of the<br>
> >>>>>> SimpleResource directly in the wrapper,<br>
> >>>>>> and the << operator for the graph.<br>
> >>>>>><br>
> >>>>>> So we would go from this:<br>
> >>>>>><br>
> >>>>>> Nepomuk::SimpleResource iconRes;<br>
> >>>>>> Nepomuk::NAO::FreeDesktopIcon icon(&iconRes);<br>
> >>>>>> icon.setIconNames( QStringList() << iconName );<br>
> >>>>>> graph << iconRes;<br>
> >>>>>> res.setProperty( Soprano::Vocabulary::NAO::prefSymbol(),<br>
> >>>>>> iconRes.uri() );<br>
> >>>>>><br>
> >>>>>> to this:<br>
> >>>>>><br>
> >>>>>> Nepomuk::NAO::FreeDesktopIcon icon;<br>
> >>>>>> icon.setIconNames( QStringList() << iconName );<br>
> >>>>>> graph << icon;<br>
> >>>>>> res.setProperty( Soprano::Vocabulary::NAO::prefSymbol(),<br>
> >>>>>> icon.uri()<br>
> >>>>>> );<br>
> >>>>><br>
> >>>>> nice.<br>
> >>>>><br>
> >>>>> Cheers<br>
> >>>>> Sebastian<br>
> >>>>><br>
> >>>>>> Cheers,<br>
> >>>>>><br>
> >>>>>> Christian<br>
> >>>>>><br>
> >>>>>>><br>
> >>>>>>> Cheers,<br>
> >>>>>>> Sebastian<br>
> >>>>>>><br>
> >>>>>>> [1]<br>
> >>>>>>> <a href="http://quickgit.kde.org/?p=kde-runtime.git&a=blob&h=304db56a9aff5523a5672415550c49d4b7269c3c&hb=344806a2e378f3421399cad39a63f14a4428308b&f=nepomuk/services/storage/lib/nepomuk-simpleresource-rcgen.py">http://quickgit.kde.org/?p=kde-runtime.git&a=blob&h=304db56a9aff5523a5672415550c49d4b7269c3c&hb=344806a2e378f3421399cad39a63f14a4428308b&f=nepomuk/services/storage/lib/nepomuk-simpleresource-rcgen.py</a><br>
> >> _______________________________________________<br>
> >> Nepomuk mailing list<br>
> >> <a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a><br>
> >> <a href="https://mail.kde.org/mailman/listinfo/nepomuk">https://mail.kde.org/mailman/listinfo/nepomuk</a><br>
> > _______________________________________________<br>
> > Nepomuk mailing list<br>
> > <a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a><br>
> > <a href="https://mail.kde.org/mailman/listinfo/nepomuk">https://mail.kde.org/mailman/listinfo/nepomuk</a><br>
><br>
><br>
> --<br>
> Using Opera's revolutionary email client: <a href="http://www.opera.com/mail/">http://www.opera.com/mail/</a><br>
> _______________________________________________<br>
> Nepomuk mailing list<br>
> <a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a><br>
> <a href="https://mail.kde.org/mailman/listinfo/nepomuk">https://mail.kde.org/mailman/listinfo/nepomuk</a><br>
</p>