calendar.h starts with this comment:

TODO: KDE5:

This API needs serious cleaning up:
- Most (all) methods aren't async ( deleteIncidence(), addIncidence(), load(), save(), ... )
  so it's not very easy to make a derived class that loads from akonadi.

- It has too many methods. Why do we need fooEvent()/fooJournal()/fooTodo() when fooIncidence()
  should be enough.

Related: Use hierarchical dptrs in KCalendarCore::Incidence classes

Many calendar component properties are interrelated, and an API that provides individual setters for them is likely to allow invalid combinations, or to have subtle behaviors depending on the order of calls. See KCalendarCore MR 45 which handles some interactions between "completed" status, completion dates, and completion percentages. An interface that sets several properties at once could be less error-prone, but I don't know how that interacts with Q_PROPERTY().

• IncidenceBase
  • d should be a unique_ptr.

• Incidence
  • GEO
    • Latitude and longitude should both be valid, or both should be undefined.
    • setHasGeo(true) is a weird thing to do.
    • Suggestions:
      • Remove setHasGeo(), setGeoLatitude(), setGeoLongitude().
      • Add
        • void setGeo(float, float); or
        • void setGeo(std::pair<float,float>); or
        • void setGeo(QVector<float>); (must have 2 elements) or
        • void setGeo(QGeoCoordinate) or
        • class Geo; void setGeo(Geo);, where Geo encapsulates the float pair and the constructor performs range checks.
      • hasGeo() is true if and only if both floats are in range.
      • geo() returns a std::pair, QVector, or Geo, to match setGeo().
  • PRIORITY
    • Defined by RFC 5545 as 0 (undefined), 1 (highest), 2, 3, ... 9 (lowest). Importance is mostly the reverse of the numerical order.
      • In a UI that can sort by priority, is highest > lowest? In KF5, no. I think KF6 should change that.
      • Is undefined < lowest? In KF5, yes. I think KF6 should keep that.
  • RELATED-TO: this should allow more than one GUID per relationship type.
    • QMultiMap<RelType,Guid> relatedTo(); or
    • QList<Guid> relatedTo(RelType);
  • Revision: rename to RFC5545 terminology SEQUENCE.
  • SEQUENCE: should this be unsigned instead of int?
  • STATUS
    • Not all status values are valid for all components.
    • Setting a status can have side-effects: changing the status of a completed should clear any completed date.
    • Suggestions:
      • Remove Incidence::setStatus() and add setStatus() to the descendant classes, or
      • Make Incidence::setStatus() virtual.