Hey Sebastian!<br><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
the patch looks pretty good. I have only two comments:<br>
1. I think having ref(), deref(), and cnt() inline is still a good idea<br>
since they are called fairly often.<br>
</blockquote><div><br>Done!<br> <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">2. Don&#39;t we need to protect m_resources via a QMutex?<br>

</blockquote><div><br>I&#39;m currently blissfully unaware about concurrent programming. I assure you I&#39;m trying to learn, but till then could you take care of it? <br> <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
[3. Wouldn&#39;t shoudBeDeleted() make more sense in ResourceManagerPrivate?]<br>
<br></blockquote><div>:-/ It checks if a ResourceData should be deleted. I considered making it static but this approach had less clutter. Could you please explain why it should be in ResourceManagerPrivate?<br> <br></div>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
As for ResourceManagerPrivate: this is just how it is done in KDE: it is<br>
a convention to name private classes that way. You can look in KDE and<br>
Qt code all over the place and find these private classes. Their main<br>
purpose is to keep as much member variables and methods out of the main<br>
class as possible to make it possible to add and remove those without<br>
breaking binary compatibility.<br></blockquote><div><br>Yea. I know. The &quot;pimpl&quot; idiom. :) <br><br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

I will apply the patch and test it.<br>
<br></blockquote><div>Okay. I&#39;ll do the same.<br><br>- Vishesh Handa<br> <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Cheers,<br>
<font color="#888888">Sebastian<br>
</font><div><div></div><div class="h5"><br>
On 05/16/2010 04:11 PM, Vishesh Handa wrote:<br>
&gt; Cleaned up the code a little bit.<br>
&gt; - fixed a small bug in Resource::~Resource(). (my fault)<br>
&gt; - m_resources is now managed along with reference counting.<br>
&gt; - Added a function ResourceData::shouldBeDeleted() to avoid duplication<br>
&gt; of code.<br>
&gt;<br>
&gt; I think the shouldBeDeleted() function should be in another patch.<br>
&gt;<br>
&gt; Btw, this code still hasn&#39;t been extensively tested. I&#39;ll do the testing<br>
&gt; later on. I just though I should share the patch so that I can get your<br>
&gt; comments.<br>
&gt;<br>
&gt; - Vishesh Handa<br>
</div></div></blockquote></div><br>