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

[Nicolas Fella]

nicolasfella added inline comments.

INLINE COMMENTS

> telephonyMessage.h:27
> +
> +class telephonyMessage: public QMap<QString, QString> {
> +public:

In general you should favor composition over inheritance, you should read about that a bit.
Meaning: TelephonyMessage should //have// a QMap to store the information, not //be// one. The public interface to TelephonyMessage should be independent of the underlying data structure, because our needs could change.

That said, I don't quite get it's purpose. Does it store a single message like the name suggests? In that case a QMap doesn't make any sense to me. A class with QStrings address and  body should be enough. If it is intended to store multiple messages the naming should reflect that. Also a QList or QMap in the TelephonyPlugin should be enough

REPOSITORY
  R224 KDE Connect

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

To: sredman, #kde_connect
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/0c9e70d4/attachment.html>