D19887: Proposal for KFileItem to skip stat()

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  The use case for this needs to be documented in the constructor. I am not sure I am in favour of this because it breaks mimetype determination from contents, which is part of the shared mime info spec.
  But actually if that's the only thing that this breaks, and there's a good use case for it, then the parameter should be SkipMimetypeDetermination. Tell the user of the API what they'll miss, not what internal implementation detail this changes.
  Everyone wants to skip a syscall, but they need to realize what this means in terms of feature loss.

INLINE COMMENTS

> kfileitem.cpp:183
> +     */
> +    bool m_bSkipStat: 1;
>  };

move it so it's with the other bits above

> kfileitem.cpp:192
>      // stat() local files if needed
>      // TODO: delay this until requested
>      if (m_fileMode == KFileItem::Unknown || m_permissions == KFileItem::Unknown || m_entry.count() == 0) {

depending on the use case, wouldn't it be enough to implement this TODO instead?

> kfileitem.cpp:751
> +                if (scheme.startsWith(QLatin1String("http")) || scheme == QLatin1String("mailto"))
> +                    d->m_mimeType = db.mimeTypeForName(QLatin1String("application/octet-stream"));
> +

missing "else" after this line?

> kfileitem.cpp:1529
> +                    d->m_mimeType = db.mimeTypeForName(QLatin1String("application/octet-stream"));
> +
> +                d->m_mimeType = db.mimeTypeForFile(url.path(), QMimeDatabase::MatchMode::MatchExtension);

same here -- duplicated code => move to helper function

> kfileitem.h:118
>  
> +    KFileItem(const QUrl &url, bool skipStat);
> +

booleans parameters are bad, please make it an enum.
https://wiki.qt.io/API_Design_Principles#The_Boolean_Parameter_Trap

+ docu missing including @since tag.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D19887

To: hoffmannrobert, dfaure, #frameworks, #dolphin
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190321/30b412cc/attachment.htm>