D11698: Add helper for reading SMS messages

Matthijs Tijink noreply at phabricator.kde.org
Tue Mar 27 19:06:27 UTC 2018


mtijink added inline comments.

INLINE COMMENTS

> SMSHelper.java:44
> +     */
> +    protected static Uri getSMSURIBad() {
> +        return Uri.parse("content://sms/");

`getSMSURIBad`? I think `getSMSURIBase` is more logical. Also, these can easily be inlined in the `getSMSUri` function.

> SMSHelper.java:76
> +     */
> +    public static Map<String, List<Map<String, String>>> getSMS(Context context) {
> +        HashMap<String, List<Map<String, String>>> toReturn = new HashMap<>();

The return type could use some improvements. I'd suggest a data class holding these result with logical names.

Also, `getSMS` is not really discriptive for what the function actually does (getting SMS threads).

> SMSHelper.java:103
> +                if (! toReturn.containsKey(thread))
> +                {
> +                    toReturn.put(thread, new ArrayList<Map<String, String>>());

Android coding style.

REPOSITORY
  R225 KDE Connect - Android application

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

To: sredman
Cc: mtijink, #kde_connect, 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/20180327/919e572c/attachment-0001.html>


More information about the KDEConnect mailing list