D13670: Add subseq operator to match sub-sequences

David Faure noreply at phabricator.kde.org
Sat Jul 7 23:37:42 BST 2018


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> ktraderparsetree.cpp:443
> +
> +  QString::Iterator i = c2.str.begin(), j = c1.str.begin();
> +  for (; i != c2.str.end() && j < c1.str.end(); ++i) {

Nothing is being modified, so this should use constBegin(), constEnd(), and const_iterator.

> ktraderparsetree.cpp:444
> +  QString::Iterator i = c2.str.begin(), j = c1.str.begin();
> +  for (; i != c2.str.end() && j < c1.str.end(); ++i) {
> +    if ((chk_case && *i == *j) || (!chk_case && i->toLower() == j->toLower())) {

This code (the actual algorithm) should be extracted into a function, and a unittest should be written for it (to make sure all corner cases are correctly handled).

Good opportunity to document the kind of matching being done, too, as you had to explain to me in this review request.

And then once this commit is in, it should be documented on https://techbase.kde.org/Development/Tutorials/Services/Traders, please remember to do so.

REPOSITORY
  R309 KService

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

To: michaeleden, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180707/4427cc69/attachment.html>


More information about the Kde-frameworks-devel mailing list