Idea: don't use QVariant::Type for storing field types

[nyalldawson]

And instead use our own enum of supported field types. Here's the reasons this may be a good move:

• QVariant::Type is full of things we will never support in fields, like QBrush and QSize. By instead explicitly listing a number of supported field types we lower the risk of bugs caused by routines not correctly handling all the supported QGIS types. Eg we often get bugs because LongLong or ULongLong or UInt aren't treated as numeric values and fall back to string handling. Having an enum of supported values gives a clear guide as to what types need to be handled in your code.
• If we add custom QVariant types as supported field types then their type will always be constant - QVariant::UserType. This makes it impossible to specify custom types as field types. I'm working towards allowing storage of geometries as an attribute, and we may want to support additional types like QgsInterval in future (since Postgres, possibly other providers, natively support interval types). For 2.x I'll have to add an "extended type" attribute to fields so the logic is "if type() == UserType then check extendedFieldType". This isn't very nice API!
• It's nicer API for python developers. Instead of needing to compare field types with QVariant types they can just refer to QgsField.FieldType for valid values.

[m-kuhn]

I haven't checked the code but I'm pretty sure it would avoid bugs like this: http://hub.qgis.org/issues/15367

[nyalldawson]

I haven't checked the code but I'm pretty sure it would avoid bugs like this: http://hub.qgis.org/issues/15367

Yes, that's exactly the type of bug this is intended to avoid.

@wonder-sk I'm keen to hear your thoughts on this before I proceed

[wonder-sk]

Is the idea to just switch from QVariant::Type to a constrained enumeration of possible types (and keep QVariant as the storage) or did you mean to replace QVariant completely with our own variant type tailored for attribute values?

[nyalldawson]

the idea to just switch from QVariant::Type to a constrained enumeration of possible types (and keep QVariant as the storage)

Yes, so a subset of Qt's inbuilt types + sensible custom types we've registered as metaphors.

did you mean to replace QVariant completely with our own variant type tailored for attribute values

Oh hell no...! I think I've just done enough search and replace to last me till the next api break :p

[3nids]

@nyalldawson if you're not a pure terminal freak, I like regexxer a lot!

[wonder-sk]

@nyalldawson Sounds good to me...

[nyalldawson]

Here's the ones I'd keep:

• Bool
• Char
• Date
• DateTime
• Double
• Int
• LongLong
• String
• Time
• UInt
• ULongLong

Agreed?

[m-kuhn]

Why not merge Int and LongLong UInt and ULongLong ? And maybe even Time and DateTime (nah, forget about that)?

[nyalldawson]

I'd be in favour, but I guess it depends on whether those type differences are important to providers. Time and datetime are very different (eg tab files, postgres types), so I don't want to merge those.

[nyalldawson]

Oh BTW, I was thinking we should have a QgsFields::isNumeric( int index ) constand bool QgsField::isNumeric() const method too.

[wonder-sk]

The list of types looks good, I would probably just skip UInt and ULongLong to keep things simpler...

[m-kuhn]

What do you think about leaving the current type information in place and rename QgsField::type() to QgsField::complexType() while offering a second method QgsField::simpleType() (which internally has a switch over the complex type)?