[Merge] lp:~ahayzen/ubuntu-settings-components/defaults-emit-to-qml-printermodel-getter-and-multi-remove-fix into lp:~phablet-team/ubuntu-settings-components/printer-components

Konrad Zapałowicz konrad.zapalowicz at canonical.com
Thu Jan 19 12:12:35 UTC 2017


Review: Needs Information code

LGTM however it seems like the getter could be const. Is that possible?

Diff comments:

> === modified file 'plugins/Ubuntu/Settings/Printers/models/printermodel.cpp'
> --- plugins/Ubuntu/Settings/Printers/models/printermodel.cpp	2017-01-18 16:25:02 +0000
> +++ plugins/Ubuntu/Settings/Printers/models/printermodel.cpp	2017-01-19 11:49:51 +0000
> @@ -305,16 +307,18 @@
>      return names;
>  }
>  
> -QSharedPointer<Printer> PrinterModel::get(const int index)
> +QVariantMap PrinterModel::get(const int row)

As a getter, shouldn't it be const?

>  {
> -    Q_D(const PrinterModel);
> +    QHashIterator<int, QByteArray> iterator(roleNames());
> +    QVariantMap result;
> +    QModelIndex modelIndex = index(row, 0);
>  
> -    if (index > -1 && index < d->printers.count()) {
> -        return d->printers.at(index);
> -    } else {
> -        qWarning() << "Invalid index:" << index;
> -        return QSharedPointer<Printer>(Q_NULLPTR);
> +    while (iterator.hasNext()) {
> +        iterator.next();
> +        result[iterator.value()] = modelIndex.data(iterator.key());
>      }
> +
> +    return result;
>  }
>  
>  QSharedPointer<Printer> PrinterModel::getPrinterFromName(const QString &name)
> @@ -333,6 +337,20 @@
>  
>  }
>  
> +QVariantMap PrinterFilter::get(const int row)

Same as above, shouldn't it be const?

> +{
> +    QHashIterator<int, QByteArray> iterator(roleNames());
> +    QVariantMap result;
> +    QModelIndex modelIndex = index(row, 0);
> +
> +    while (iterator.hasNext()) {
> +        iterator.next();
> +        result[iterator.value()] = modelIndex.data(iterator.key());
> +    }
> +
> +    return result;
> +}
> +
>  void PrinterFilter::filterOnState(const PrinterEnum::State &state)
>  {
>      Q_UNUSED(state);


-- 
https://code.launchpad.net/~ahayzen/ubuntu-settings-components/defaults-emit-to-qml-printermodel-getter-and-multi-remove-fix/+merge/315114
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/ubuntu-settings-components/printer-components.



More information about the Ubuntu-reviews mailing list