[Qt-interest] Mismatch between QSpinBox::textFromValue() documentation and actual behavior

[Anders Bakken]

On Wed, Mar 24, 2010 at 01:41:48AM +0100, ext K. Frank wrote:
> Hello List -

Hi Frank

> 
> (This is a follow-up on my previous posting, "How to make QSpinBox display
> thousands separators?")
> 
> I am using Qt version 4.6.1 (as built on windows using TDM-2 mingw32 4.4.1).
> 
> I find that the behavior of QSpinBox::textFromValue() does not match
> the documentation.
> 
> 
> Copied from the documentation, .../doc/html/qspinbox.html#textFromValue:
> 
> QString QSpinBox::textFromValue ( int value ) const   [virtual protected]
> This virtual function is used by the spin box whenever it needs to
> display the given value. The default implementation returns a string
> containing value printed in the standard way using
> QWidget::locale().toString(). Reimplementations may return anything.
> (See the example in the detailed description.)
> 
> Note: QSpinBox does not call this function for specialValueText() and
> that neither prefix() nor suffix() should be included in the return
> value.
> 
> If you reimplement this, you may also need to reimplement
> valueFromText() and validate()
> 
> See also valueFromText() and validate().

This is missing from the docs. It does indeed remove it. While this may
or may not be the most appropriate default it's relatively easy to work
around. 

> 
> 
> At issue is that QWidget::locale().toString(int) formats integers with
> thousands separators,
> while QSpinBox::textFromValue(int) does not.
> 
> Concretely, the following class derived from QSpinBox gives us access
> to the protected
> QSpinBox::textFromValue(int).  (This class also overrides
> textFromValue() to restore
> the thousands separators.)
> 
> 
> #include <QSpinBox>
> class MySpinBox : public QSpinBox {
>   Q_OBJECT
>    public:
>     MySpinBox (QWidget *parent = 0) : QSpinBox (parent) {}
>     ~MySpinBox() {};
>     QString getBaseTextFromValue (int value) const { return
> QSpinBox::textFromValue (value); }
>     QString getTextFromValue (int value) const { return textFromValue (value); }
>   protected:
>     QString textFromValue (int value) const { return locale().toString
> (value); }
> };
> 
> 
> I load a MySpinBox with the value 10000.
> 
>    mySpinBox->value() returns 10000
>    mySpinBox->getBaseTextFromValue (mySpinBox->value()) returns "10000"
>    mySpinBox->locale().toString (mySpinBox->value()) returns "10,000"
> 
> I would have expected these last two to return the same string because
> QSpinBox::textFromValue() -- called by MySpinBox::getBaseTextFromValue() --
> should be equivalent to QWidget::locale().toString().
> 
The problem is that getBaseTextFromValue explicitly calls
QSpinBox::textFromValue() which means it will go into this function:

QString QSpinBox::textFromValue(int value) const 
{ 
    QString str = locale().toString(value); 
    if (qAbs(value) >= 1000 || value == INT_MIN) { 
        str.remove(locale().groupSeparator()); 
    } 

    return str; 
} 

Rather than your reimplementation. If you change it like so:
     QString getBaseTextFromValue (int value) const { return textFromValue(value); }

It will work like you expected.

> (Furthermore,
> 
>    mySpinBox->text() returns "10,000"
>    mySpinBox->getTextFromValue (mySpinBox->value()) returns "10,000"
> 
> because these two resolve to MySpinBox::textFromValue(), the overridden
> version of textFromValue() in my derived class.  So this is my solution to
> getting thousands separators in my spin box.)
> 
> I don't know what the desired behavior is supposed to be, so I don't know
> whether this is a documentation bug or and implementation bug.
> (It's interesting to note that the documentation for
> QDoubleSpinBox::textFromValue() states explicitly that thousands
> separators are removed.)
> 
> (My personal preference for the behavior would be to support both the
> presence and absence of thousands separators, presumably by
> setting an options flag (or maybe in a tweak to locale), and not have
> to resort to the minor effort of setting up a derived class.)
> 
> Thanks.
> 
> 
> K. Frank

I've pushed a change to fix the docs. It could maybe be argued that this
should be available as an option of some sort. It's relatively simple to
work around though.

Thanks for the report.

regards

-- 
Qt Development Frameworks, Nokia, http://qt.nokia.com
Anders Bakken