[PATCH] remove UDSEntry from the public KIO API?

nf2 nf2 at scheinwelt.at
Mon Jul 23 19:11:46 BST 2007


Jeff Mitchell wrote:
> Thiago Macieira wrote:
>> nf2 wrote:
>>  
>>> Thiago Macieira wrote:
>>>    
>>>> This hides the details of UDSEntry and UDSField from the public. The
>>>> question is now? Should we commit? Norbert, does it help towards your
>>>> goal?
>>>>       
>>> Cool. I have seen you have also fixed the KioSlaveTest (which i have
>>> just commented out). And udsentry.cpp looks a lot nicer now.
>>>
>>> I think the patch should be commited - everything which makes the
>>> KIO-API more generic and extensible (without having to break the API)
>>> will help you in the future. No matter if the future is GVFS or not.
>>>     
>>
>> I'm not doing a "commit if no objections" on this one.
>>
>> Someone else please OK this within the next 12 hours, or this patch 
>> will be out of KDE until KDE 5.0.
>>
>> For ease of reading the patch, here's the most relevant part.
>>
>>   
> I'm currently working with UDSEntry and having taken a look at the 
> diff, the one question I have is why there's no operator[].  Norbert 
> said:  "Regarding the operator[] i'm not sure - i would prefer a 
> method which returns an array of ints (a QList<uint> 
> getListOfFieldIds()) because there might be QList<QString> 
> getListOfFieldNames() in the future..."
>
> Is there a reason not to have both?  I know that it's no longer a 
> QHash derivative (since that's in the private class now), but it would 
> help ensure ease of transitioning to the new code/backwards 
> compatibility.
>
The problem with the operator[] is that it has to return a complete 
Field item - which has been an instance of UDSField in the past:

class KIO_EXPORT UDSField
{
public:
  UDSField() {}
  UDSField( const QString& s ) : m_str( s ) {}
  UDSField( long long l ) : m_long( l ) {}
  QString toString() const { return m_str; }
  long long toNumber() const { return m_long; }
private:
  QString m_str;
  long long m_long;
};

Exposing UDSField in the public API makes it impossible to put more 
complex field information into it in the future. For instance this comment

    /// The access control list serialized into a single string.
    UDS_ACL_STRING = 20 | UDS_STRING,

shows that people already had to work around those limitations...

In the entire kdepimlibs and kdebase the operator[] is never used. Also, 
there are no iterators over the QHash. Therefore i wouldn't expect many 
compatibility issues.

Norbert








More information about the kde-core-devel mailing list