Made Paintable::DrawMode an enum class #13424
Conversation
| @@ -16,7 +16,7 @@ class QSvgRenderer; | |||
| // high fidelity. | |||
| class Paintable { | |||
| public: | |||
| enum DrawMode { | |||
| enum class DrawMode { | |||
There was a problem hiding this comment.
if this change results in a touch to every point of use anyways, can you also change the members to TitleCase?
There was a problem hiding this comment.
I noticed, that the upper case strings are used accross the skin XML files. Therefore the string must remain upper case for backward compatibility.
PS: I just added a debug assert, which worked;-)
There was a problem hiding this comment.
Now we have to decide, should we switch to Q_ENUM or should we rename to TitelCase?
Since Q_ENUM adds a overhead, I would prefer to rename and implement these two functions otherwise.
There was a problem hiding this comment.
Since Q_ENUM adds a overhead
Out of curiosity, in what sense? From what I can tell, it only adds a little bit of compilation time due to the MOC.
There was a problem hiding this comment.
Yes, the class must be a QObject and the MOC compilation comes on top.
There was a problem hiding this comment.
The QObject could be avoided if the enum was moved to a namespace technically. Though I'm fine with leaving it as a plain enum class for now. Its unlikely that this enum would need to get exposed to the QML environment anyways (which is where the biggest value of a QEnum lies IMO).
There was a problem hiding this comment.
This enum is only used in our legacy widgets. A use with QML is highly unlikely.
No description provided.