D11854: (WIP) Upgrade Telephony plugin to read SMS history (KDE side)

[Nicolas Fella]

nicolasfella added a comment.


  Looking much better, but there is still room for improvement. Passing a QVariantMap in the constructor is IMHO a violation of the law of demeter. The constructor should not make assumptions/rely on the content of args. It should ask directly for what it wants, that is a String body and a String address. I assume you want to be extensible this way, but adding a third property, say an image that is being sent would still need changes in the class. That would be a violation of the open-closed principle, that states that a class should be closed for modification but open for extension. Taking the example of an image message: Instead of cramming all the logic into Message one should create a class ImageMessage that extends Message and takes the image as a third constructor argument.

REPOSITORY
  R224 KDE Connect

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

To: sredman, #kde_connect, nicolasfella
Cc: nicolasfella, #kde_connect, Danial0_0, krantideep95, Pitel, adeen-s, SemperPeritus, ahmedbesbes, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, ach, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180401/34ef25dd/attachment.html>