[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