[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