It is possible that I miss something really obvious. But could you please explain me in more details what is wrong if we do like this:<br><br>1) Make SimpleResource explicitly shared<br>class SimpleResourcePrivate;<br>class SimpleResource {<br>
<br>.....<br>private:<br>QExplicitlySharedDataPointer<SimpleResourcePrivate> d;<br>friend class WrapperBase;<br>}<br><br>and use detach in all non-const methods<br><br>2) Create a base class for all autogenerated wrappers.<br>
class WrapperBase <br>{<br>public constructors:<br>WrapperBase( SimpleResource & )<br><br>WrapperBas()<br><br>private constructors:<br>WrapperBase( QExplicitlySharedDataPointer<SimpleResourcePrivate> & resource )<br>
<br>other methods:<br>SimpleResource resource() const;<br><br>protected:<br>QExplicitlySharedDataPointer<SimpleResourcePrivate> m_resource;<br> <br>};<br><br>and inherit all other classes from this class virtually if the do not have other base classes<br>
<br>class InformationElement : virtual public WrapperBase<br>{<br>// Methods of InformatonElement here. They have access to m_resource to make changes<br>};<br><br>class TvSeries : InformationElement, SomeOtherBaseClass<br>
{<br>// Methods of TvSeries here<br>};<br><br>And realizaton of <br>SimpleResource WrapperBase::resource() const<br>{<br>return SimpleResource(m_resource);<br>}<br><br><div class="gmail_quote">On Wed, Jul 27, 2011 at 10:58 AM, Sebastian Trüg <span dir="ltr"><<a href="mailto:trueg@kde.org">trueg@kde.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im">On 07/27/2011 08:14 AM, Artem Serebriyskiy wrote:<br>
><br>
> 27.07.2011 3:12 пользователь "Christian Mollekopf" <<a href="mailto:chrigi_1@fastmail.fm">chrigi_1@fastmail.fm</a><br>
</div>> <mailto:<a href="mailto:chrigi_1@fastmail.fm">chrigi_1@fastmail.fm</a>>> написал:<br>
<div class="im">>><br>
>> On Tue, 26 Jul 2011 20:57:46 +0200, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a><br>
</div><div class="im">> <mailto:<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>
</div>> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>><br>
<div class="im">>> >> 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>
</div>> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>><br>
<div class="im">>> >>>> 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<br>
</div>> <<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>><br>
<div><div></div><div class="h5">>> >>>>>> 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<br>
> 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<br>
> 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<br>
> 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<br>
> to SimpleResource::Private( and make it less private, of course ) ? This<br>
> way Contact c(r); will share with r it's privates, Contact c; will<br>
> create new private and in all cases c.resource(); will detach a private<br>
> and create a new SimpleResource with detached private copy?<br>
<br>
</div></div>I think this results in the same multi-inheritance problem and results<br>
in very ugly memory maintenance. We cannot use protected member<br>
variables in the base classes since the compiler will not know which to<br>
use when inheriting from multiple classes.<br>
<br>
Thus, I think we are left with the only option that I found originally:<br>
plain wrappers without any convenience.<br>
<br>
Even code like<br>
Contact c;<br>
c.setFullname(...);<br>
SimpleResource res = c.resource();<br>
would require ugly pointer stuff which would look something like<br>
<br>
class Contact : public Role, public Party {<br>
public:<br>
Contact()<br>
: m_res(new SimpleResource()),<br>
m_deleteMe(true) {<br>
Role::setRes(m_res);<br>
Party::setRes(m_res);<br>
}<br>
~Contact() {<br>
if(m_deleteMe) delete m_res;<br>
}<br>
SimpleResource resource() const {<br>
return *m_res;<br>
}<br>
void setRes(SimpleResource* res) {<br>
if(deleteMe) delete m_res;<br>
m_res = res;<br>
Role::setRes(res);<br>
Party::setRes(res);<br>
}<br>
<br>
private:<br>
SimpleResource* m_res;<br>
bool m_deleteMe;<br>
};<br>
<br>
Hm, maybe that is even doable....<br>
<br>
Cheers,<br>
Sebastian<br>
<div><div></div><div class="h5"><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>
>> >>>>>>><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" target="_blank">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>
> <<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" target="_blank">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>
</div></div>>> >> <a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a> <mailto:<a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a>><br>
<div class="im">>> >> <a href="https://mail.kde.org/mailman/listinfo/nepomuk" target="_blank">https://mail.kde.org/mailman/listinfo/nepomuk</a><br>
>> > _______________________________________________<br>
>> > Nepomuk mailing list<br>
</div>>> > <a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a> <mailto:<a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a>><br>
<div class="im">>> > <a href="https://mail.kde.org/mailman/listinfo/nepomuk" target="_blank">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/" target="_blank">http://www.opera.com/mail/</a><br>
>> _______________________________________________<br>
>> Nepomuk mailing list<br>
</div>>> <a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a> <mailto:<a href="mailto:Nepomuk@kde.org">Nepomuk@kde.org</a>><br>
<div><div></div><div class="h5">>> <a href="https://mail.kde.org/mailman/listinfo/nepomuk" target="_blank">https://mail.kde.org/mailman/listinfo/nepomuk</a><br>
><br>
><br>
><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" target="_blank">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" target="_blank">https://mail.kde.org/mailman/listinfo/nepomuk</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Sincerely yours,<br>Artem Serebriyskiy<br>