D20881: KCalCore Clazy fixes

[Daniel Vrátil]

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

INLINE COMMENTS

> occurrenceiterator.cpp:126
>                  QDateTime occurrenceStartDate;
> -                for (auto recurrenceId : qAsConst(occurrences)) {
> +                for (auto &recurrenceId : qAsConst(occurrences)) {
>                      occurrenceStartDate = recurrenceId;

Since you are touching this, should it be `const auto &` for clarity? Also couple more times below in this patch.

> recurrencerule.cpp:2078
> +            }
> +            lst.append(bydays);
>          }

Should this be in an `else` branch? Otherwise `bydays` will be appended twice if the condition above is true

> vcalformat.cpp:363
> +                recurrenceTypeAbbrLen = 2;
>                  if (tmpStr.left(2) == QStringLiteral("MP")) {
> +                    recurrenceType = Recurrence::rMonthlyPos;

You could use `leftRef()` and compare with a `QLatin1String()` to reduce allocations, since you are touching this code.

> vcalformat.cpp:788
>  
> -    // repeat stuff
> +    // recurrence stuff
>       if ((vo = isAPropertyOf(vevent, VCRRuleProp)) != nullptr) {

This looks identical to the code above, should it be moved into its own function? (not necessarily in this review, but if you feel bored ;-))

> vcalformat.cpp:807
> +                recurrenceTypeAbbrLen = 2;
>                  if (tmpStr.left(2) == QLatin1String("MP")) {
> +                    recurrenceType = Recurrence::rMonthlyPos;

Same as above.

REPOSITORY
  R172 KCalendar Core

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

To: winterz, vkrause, mlaurent, dvratil
Cc: dvratil, kde-pim, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20190428/43de7671/attachment.html>