[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Comparing KFileItems
From:       David Faure <faure () kde ! org>
Date:       2012-10-24 16:21:45
Message-ID: 5236253.LEufPzeJi0 () asterix ! site
[Download RAW message or body]

On Wednesday 24 October 2012 10:47:46 Frank Reininghaus wrote:
> Hi David,
> 
> I see that I should probably have created a review request to make
> review easier for you - sorry about that! But I think we're getting
> closer to the final solution, so I'll just reply to your message with
> a new patch.

Heh, if you're doing all the work, who am I to complain about the way you send the \
patch? :-) No problem at all.  
> > Or return !operator==(other), inline, for easier maintainance.
> 
> I haven't implemented the inline part yet. Would you prefer to have
> the inline method inside the class definition or rather just move the
> function into the header file and prepend it with 'inline'? 

I was thinking the first one, but indeed Qt often does the second one,
I'm not sure what the difference really is. Either one is fine with me.


     /**
      * Somewhat like a comparison operator, but more explicit,
      * and it can detect that two kfileitems are equal even when they do
-     * not share the same internal pointer - e.g. when KDirLister compares
+     * not share the same URL - e.g. when KDirLister compares
      * fileitems after listing a directory again, to detect changes.
      * @param item the item to compare
      * @return true if all values are equal
      */
     bool cmp( const KFileItem & item ) const;

Hmm, this doesn't make sense anymore :-)
In KDirLister the item doesn't change URL.
I would suggest:
     /**
      * Somewhat like a comparison operator, but more explicit,
      * and it can detect that two fileitems differ if any property of the file item
      * has changed (file size, modification date, etc.). Two items are equal if
      * all properties are equal. In contrast, operator== only compares URLs.
      * @param item the item to compare
      * @return true if all values are equal
      */
     bool cmp( const KFileItem & item ) const;

Thanks for the fix, please commit to 4.10.
(This changes behavior, so it could create bugs in existing code, let's not commit
it in 4.9 IMHO)

-- 
David Faure, faure@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5


[prev in list] [next in list] [prev in thread] [next in thread]