D6820: Add QValidator to KTimeCombobox

[Christoph Feck]

cfeck requested changes to this revision.
cfeck added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> ktimecombobox.cpp:33
> +public:
> +    KTimeValidator(QString timeFormat, QObject *parent = Q_NULLPTR);
> +    KTimeValidator(QString timeFormat, QTime min, QTime max, QObject *parent = Q_NULLPTR);

Please use "const &" for all QTime and QString arguments.

> ktimecombobox.cpp:34
> +    KTimeValidator(QString timeFormat, QObject *parent = Q_NULLPTR);
> +    KTimeValidator(QString timeFormat, QTime min, QTime max, QObject *parent = Q_NULLPTR);
> +    ~KTimeValidator();

Since "min" and "max" might be macros for some compilers, please rename to "minTime" and "maxTime".

> ktimecombobox.cpp:37
> +    virtual State validate(QString &input, int &pos) const Q_DECL_OVERRIDE;
> +    void setMin(QTime min);
> +    void setMax(QTime max);

setMinTime(const QTime &minTime);

It looks more verbose, but also more correct :)

> ktimecombobox.cpp:42
> +    QString m_timeFormat;
> +    QTime m_min, m_max;
> +};

Also here, m_minTime and m_maxTime.

> ktimecombobox.cpp:76
> +    Q_UNUSED(pos);
> +    QTime t = QTime::fromString(input, m_timeFormat);
> +    if (t.isValid()) {

const QTime time = ...

> ktimecombobox.cpp:231
> +    m_validator = new KTimeValidator(locale.timeFormat(m_displayFormat),
> +                                     m_minTime, m_maxTime);
> +    q->lineEdit()->setValidator(m_validator);

So the KTimeComboBox class already has m_min/maxTime members. Can the code be refactored that they do not need to be stored twice?

REPOSITORY
  R236 KWidgetsAddons

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

To: bednar, cfeck
Cc: cfeck, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170803/8304ac94/attachment-0001.html>