D13358: Add new class that is a model of numbers between two values

[Mark Gaiser]

markg added a comment.


  This basically is the equivalent of the C++ range iterator (https://github.com/ryanhaining/cppitertools#range, but more libraries have a "range" iterator like that).
  
  In all the documentation blocks you miss the argument and return value documentation.
  
  I'm curious though, in which situation did you need a model like this?
  A situation i can think of is where you would always want to show x number of items (say for instance a top 10 downloads) and just leve the entries blank (but have the index numbers) if there isn't enough to fill the top 10.

INLINE COMMENTS

> knumbermodel.cpp:24
> +#include <QLocale>
> +#include <QDebug>
> +

QDebug is unused.

> knumbermodel.cpp:34
> +    qreal step = 1.0;
> +    QLocale::NumberOptions  formattingOptions = QLocale::DefaultNumberOptions;
> +};

Drop one space..

> knumbermodel.cpp:114
> +{
> +    return d->min + d->step * index.row();
> +}

This is only guaranteed to be a valid index if the callers call it with a valid index.
Better be safe then sorry and check for isValid().

> knumbermodel.cpp:121
> +    }
> +    return std::max(0.0, std::floor((d->max - d->min) / d->step)) + 1;
> +}

Won't this give compile warnings? It's double and int foo mixed.
Also, the +1 should probably be "+ d->step"

> knumbermodel.cpp:140-143
> +    QHash<int, QByteArray> roleNames;
> +    roleNames[Display] = QByteArrayLiteral("display");
> +    roleNames[Value] = QByteArrayLiteral("value");
> +    return roleNames;

This can be compile-time generated.

REPOSITORY
  R275 KItemModels

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

To: davidedmundson
Cc: markg, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180605/7f095e00/attachment-0001.html>