Re: [Development] Naming convention for (scoped) enums
Seems that there is no definite consensus on this, but we will abandon https://codereview.qt-project.org/235167, since it's not that important for us in the end. It seems that most people are against such a change. It would be good if a decision is taken on this (and wiki is updated) in order to not have this discussion again later. Jan Arve Fra: Development på vegne av Simon Hausmann Sendt: mandag 3. september 2018 16.31.07 Til: development@qt-project.org Emne: Re: [Development] Naming convention for (scoped) enums Am 31.08.18 um 11:56 schrieb Tor Arne Vestbø: > I think Simon’s reasoning in the review that spurred this discussion > summarises it nicely: > >> On 31 Aug 2018, at 10:24, Simon Hausmann (Code Review) >> wrote: >> >> Simon Hausmann has posted comments on this change. >> >> Change subject: Convert QQEventPoint and QQPointerDevice enums to enum >> classes >> .. >> >> >> Patch Set 7: >> >> As excited I was initially with enum classes, I also start to dislike them >> when looking at their use. >> >> The counter example, QQuickPointerDevice::Mouse, is awesome. >> QQuickPointerDevice::DeviceType::Mouse looks worse. >> >> Always scoping leads to redundancy and never scoping leads to clashes. Enum >> classes don't allow us to choose, they force us into the longer names. The >> previous policy of prefixing _when needed_ gave us the flexibility to have >> lean names when we could and longer names when required. For example >> QuickItem::ItemHasContents. >> >> So in terms of naming I find enum classes not truly winning. Perhaps they >> make us more lazy in finding the best names, because just putting whatever >> we have in an enum class "takes care of it". >> >> The remaining argument in favor of enum classes is the type safety they add. >> But at least inside Qt I've often seen it become an anti-pattern because we >> do things in a more low-level fashion and need to cast to an int sometimes, >> for example. >> >> Given the names in this very API, I also disagree with commit message >> statement that the existing scoping is insufficient. (See >> QQuickPointerDevice::GrabState::GrabExclusive) > Based on the disagreement on how and when to use scoped enums, I think we > should change the style policy to: > > - always recommend using scoped enums for global enums > - describe the pro’s and con’s of scoped enums inside classes > - ask the developer to consider each case individually >- and use good judgement in choosing one or the other +1 on this. Simon ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
Am 31.08.18 um 11:56 schrieb Tor Arne Vestbø: I think Simon’s reasoning in the review that spurred this discussion summarises it nicely: On 31 Aug 2018, at 10:24, Simon Hausmann (Code Review) wrote: Simon Hausmann has posted comments on this change. Change subject: Convert QQEventPoint and QQPointerDevice enums to enum classes .. Patch Set 7: As excited I was initially with enum classes, I also start to dislike them when looking at their use. The counter example, QQuickPointerDevice::Mouse, is awesome. QQuickPointerDevice::DeviceType::Mouse looks worse. Always scoping leads to redundancy and never scoping leads to clashes. Enum classes don't allow us to choose, they force us into the longer names. The previous policy of prefixing _when needed_ gave us the flexibility to have lean names when we could and longer names when required. For example QuickItem::ItemHasContents. So in terms of naming I find enum classes not truly winning. Perhaps they make us more lazy in finding the best names, because just putting whatever we have in an enum class "takes care of it". The remaining argument in favor of enum classes is the type safety they add. But at least inside Qt I've often seen it become an anti-pattern because we do things in a more low-level fashion and need to cast to an int sometimes, for example. Given the names in this very API, I also disagree with commit message statement that the existing scoping is insufficient. (See QQuickPointerDevice::GrabState::GrabExclusive) Based on the disagreement on how and when to use scoped enums, I think we should change the style policy to: - always recommend using scoped enums for global enums - describe the pro’s and con’s of scoped enums inside classes - ask the developer to consider each case individually - and use good judgement in choosing one or the other +1 on this. Simon ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
Enumerations inside of classes makes dependency breaking harder. We don't use that in Qt but users of our interface may do. It's still possible but much more complex because you generally get much more dependencies. A simple header file for enumerations instead can simply be included without any additional dependencies. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
Hey, > On 31 Aug 2018, at 14:50, Jan-Arve Sæther wrote: > > For me it seems that enum classes can help us create a more structured API, > because they also gives more semantic information, e.g: > QQuickPointerDevice::PointerType::Finger > gives more information than > QQuickPointerDevice::Finger Again, showing the enum like this leaves out context that is usually there in real code: if (ev->pointerType() == QQuickPointerDevice::Finger) At which point the “more semantic information”, becomes “redundant semantic information”: if (ev->pointerType() == QQuickPointerDevice::PointerType::Finger) > For example, when doing code completion if I type "QQuickPointerDevice::" it > will give me _all_ members of QQuickPointerDevice (which I always have found > a bit annoying), but with an enum class I can type > "QQuickPointerDevice::PointerType::" and I will get listed only the valid > values (finally). Code completion should be able to look at the type you are comparing with (as above), and list the relevant enum values first (or only). We do that for function arguments don’t we, when suggesting variables to pass e.g.? > With the extra semantic I can even assume that the associated property for > the value QQuickPointerDevice::PointerType::Finger is called pointerType > (which it is). (I admit this is not always the case, but maybe it should be a > goal). Do you often go from enum value to property name? (Related but orthogonal, I notice our docs do not point back from an enum to functions that reference it. Perhaps there should be some automatic resolving there based on properties e.g. so that one doesn’t have to do \sa manually?) > And having it up for discussion for each review whether an enum should be > class enums or not just sounds like were planning for lots of bike-shedding > ahead. Is it better to forcefully enforce a coding style that is contended, and that might hurt code readability and our APIs, just to not “have to think”? > Last, maybe enum classes will lead to more ugly static_cast inside Qt, but > that's a cheap price to pay if it gives our users a better API (my point is > that a few awkward internal static_cast should not influence how our public > API is). What if it gives our users a _worse_ API? How does that balance look? Should we pay for that too? My -5 øre 😊 Tor Arne ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
For me it seems that enum classes can help us create a more structured API, because they also gives more semantic information, e.g: QQuickPointerDevice::PointerType::Finger gives more information than QQuickPointerDevice::Finger For example, when doing code completion if I type "QQuickPointerDevice::" it will give me _all_ members of QQuickPointerDevice (which I always have found a bit annoying), but with an enum class I can type "QQuickPointerDevice::PointerType::" and I will get listed only the valid values (finally). With the extra semantic I can even assume that the associated property for the value QQuickPointerDevice::PointerType::Finger is called pointerType (which it is). (I admit this is not always the case, but maybe it should be a goal). And having it up for discussion for each review whether an enum should be class enums or not just sounds like were planning for lots of bike-shedding ahead. Last, maybe enum classes will lead to more ugly static_cast inside Qt, but that's a cheap price to pay if it gives our users a better API (my point is that a few awkward internal static_cast should not influence how our public API is). my 5 øre --- Jan Arve Sæther Senior Software Engineer Fra: Development på vegne av Tor Arne Vestbø Sendt: fredag 31. august 2018 11.56.15 Til: Alex Blasche Kopi: development@qt-project.org Emne: Re: [Development] Naming convention for (scoped) enums I think Simon’s reasoning in the review that spurred this discussion summarises it nicely: > On 31 Aug 2018, at 10:24, Simon Hausmann (Code Review) > wrote: > > Simon Hausmann has posted comments on this change. > > Change subject: Convert QQEventPoint and QQPointerDevice enums to enum classes > .. > > > Patch Set 7: > > As excited I was initially with enum classes, I also start to dislike them > when looking at their use. > > The counter example, QQuickPointerDevice::Mouse, is awesome. > QQuickPointerDevice::DeviceType::Mouse looks worse. > > Always scoping leads to redundancy and never scoping leads to clashes. Enum > classes don't allow us to choose, they force us into the longer names. The > previous policy of prefixing _when needed_ gave us the flexibility to have > lean names when we could and longer names when required. For example > QuickItem::ItemHasContents. > > So in terms of naming I find enum classes not truly winning. Perhaps they > make us more lazy in finding the best names, because just putting whatever we > have in an enum class "takes care of it". > > The remaining argument in favor of enum classes is the type safety they add. > But at least inside Qt I've often seen it become an anti-pattern because we > do things in a more low-level fashion and need to cast to an int sometimes, > for example. > > Given the names in this very API, I also disagree with commit message > statement that the existing scoping is insufficient. (See > QQuickPointerDevice::GrabState::GrabExclusive) Based on the disagreement on how and when to use scoped enums, I think we should change the style policy to: - always recommend using scoped enums for global enums - describe the pro’s and con’s of scoped enums inside classes - ask the developer to consider each case individually - and use good judgement in choosing one or the other Tor Arne ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 31. Aug 2018, at 11:56, Tor Arne Vestbø wrote: > > I think Simon’s reasoning in the review that spurred this discussion > summarises it nicely: > >> On 31 Aug 2018, at 10:24, Simon Hausmann (Code Review) >> wrote: >> >> Simon Hausmann has posted comments on this change. >> >> Change subject: Convert QQEventPoint and QQPointerDevice enums to enum >> classes >> .. >> >> >> Patch Set 7: >> >> As excited I was initially with enum classes, I also start to dislike them >> when looking at their use. >> >> The counter example, QQuickPointerDevice::Mouse, is awesome. >> QQuickPointerDevice::DeviceType::Mouse looks worse. That could be probably be QQuickPointerDevice::Type::Mouse to kill the redundancy in the name. (I wonder a bit what a device with type Mouse and pointer type Pen would look like btw.) >> >> Always scoping leads to redundancy and never scoping leads to clashes. Enum >> classes don't allow us to choose, they force us into the longer names. Which at least reduces future surprises. >> The previous policy of prefixing _when needed_ gave us the flexibility to >> have lean names when we could and longer names when required. For example >> QuickItem::ItemHasContents. >> >> So in terms of naming I find enum classes not truly winning. Perhaps they >> make us more lazy in finding the best names, because just putting whatever >> we have in an enum class "takes care of it”. And maybe not. If you use unscoped enums without prefix, then this can lead to bad names in the future when items must be added to some enum. And also maybe not. >> The remaining argument in favor of enum classes is the type safety they add. >> But at least inside Qt I've often seen it become an anti-pattern because we >> do things in a more low-level fashion and need to cast to an int sometimes, >> for example. IMO especially if you need to pass an enum to API that uses integers once in a while, it is a good thing if you need to make this conversion explicit. >> >> Given the names in this very API, I also disagree with commit message >> statement that the existing scoping is insufficient. (See >> QQuickPointerDevice::GrabState::GrabExclusive) > > Based on the disagreement on how and when to use scoped enums, I think we > should change the style policy to: > > - always recommend using scoped enums for global enums > - describe the pro’s and con’s of scoped enums inside classes >- ask the developer to consider each case individually > - and use good judgement in choosing one or the other Obviously we have a disagreement on what the “good judgement” is supposed to be though ;) -- Eike Ziller Principal Software Engineer The Qt Company GmbH Rudower Chaussee 13 D-12489 Berlin eike.zil...@qt.io http://qt.io Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
I think Simon’s reasoning in the review that spurred this discussion summarises it nicely: > On 31 Aug 2018, at 10:24, Simon Hausmann (Code Review) > wrote: > > Simon Hausmann has posted comments on this change. > > Change subject: Convert QQEventPoint and QQPointerDevice enums to enum classes > .. > > > Patch Set 7: > > As excited I was initially with enum classes, I also start to dislike them > when looking at their use. > > The counter example, QQuickPointerDevice::Mouse, is awesome. > QQuickPointerDevice::DeviceType::Mouse looks worse. > > Always scoping leads to redundancy and never scoping leads to clashes. Enum > classes don't allow us to choose, they force us into the longer names. The > previous policy of prefixing _when needed_ gave us the flexibility to have > lean names when we could and longer names when required. For example > QuickItem::ItemHasContents. > > So in terms of naming I find enum classes not truly winning. Perhaps they > make us more lazy in finding the best names, because just putting whatever we > have in an enum class "takes care of it". > > The remaining argument in favor of enum classes is the type safety they add. > But at least inside Qt I've often seen it become an anti-pattern because we > do things in a more low-level fashion and need to cast to an int sometimes, > for example. > > Given the names in this very API, I also disagree with commit message > statement that the existing scoping is insufficient. (See > QQuickPointerDevice::GrabState::GrabExclusive) Based on the disagreement on how and when to use scoped enums, I think we should change the style policy to: - always recommend using scoped enums for global enums - describe the pro’s and con’s of scoped enums inside classes - ask the developer to consider each case individually - and use good judgement in choosing one or the other Tor Arne ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> -Original Message- > From: Development project.org> On Behalf Of Simon Hausmann > > I don't consider the longer names detrimental for writability and usability. > Writability is easily solved with code completion and readability is actually > better > because the type adds additional context info. To cite an example from Shawn's > patch: > > > > QQuickPointerDevice::Finger > > QQuickPointerDevice::Mouse > > > > is far less descriptive than (and not compliant to current naming > > convention) > > > > QQuickPointerDevice::PointerType::Finger > > QQuickPointerDevice::DeviceType::Mouse. > > > > The uninitiated reader of the code cannot see the additional type context > when reading code and therefore someone might interpret Finger as a device > type too. That's exactly what the existing unscoped enum policy tries to fix. > Scoped enums provide this feature out of the box. Another example of a good > naming policy (IMO) is QAbstractSocket (never mind the technical limitations). > Here we could shorten the enum names using scoped enums: > > > > QAbstractSocket::SocketError::ConnectionRefusedError -> > QAbstractSocket::SocketError::ConnectionRefused (or even > QAbstractSocket::Error::ConnectionRefused) > > > > Name clash prevention was only ever secondary to readability. > > > We don't need scoped enums to improve readability though, that doesn't > seem like an argument. To take the finger/mouse example, we would use > > QQuickPointerDevice::FingerPointer > > or > > QQuickPointerDevice::PointerType_Finger > > and you get exactly the same benefit, no? I don't see how your two options above are any different(better?) readability wise than QQuickPointerDevice::PointerType::Finger. I believe I mentioned earlier that in light of Qt 6 it might actually make sense to rename some enum type names too which means QQuickPointerDevice::Pointer::Finger is possible and we can remove some of the redundancy between enum type name and enum value name. Sure, we can get away with the old style of naming but when you consider that the length of the string is extremely close to being the same and gain type safety, why would we not use them? Of course, if you purposely don't want to type safety traditional enums are still the way to go. My five cents.. -- Alex ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
Simon Hausmann (31 August 2018 11:01) > If we decide to allow deciding on a case-by-case basis, then I encourage > everyone to carefully look at newly introduced enums, their values and > their context in the upcoming "API review season". or, indeed, s/upcoming/ongoing/ ;-) Eddy. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
On 8/15/18 9:32 AM, Alex Blasche wrote: -Original Message- From: Tor Arne Vestbø 1. Scoped enums (enum class) for the sake of avoiding name clashes is useful for global enums, but when the enum lives inside a class, the chance that we’ll see a naming clash is minor, and using scoped enums in that case arguably has negative effects on code writability/readability. As a result, we should update the policy to not mandate scoped enums when the enum lives inside a class. I don't think we have ever not permitted exceptions to official policy. Therefore, take it for granted that the policy can be ignored such as in the case presented by Allan. Having said that the default should be the use of scoped enums even inside of classes. I don't consider the longer names detrimental for writability and usability. Writability is easily solved with code completion and readability is actually better because the type adds additional context info. To cite an example from Shawn's patch: QQuickPointerDevice::Finger QQuickPointerDevice::Mouse is far less descriptive than (and not compliant to current naming convention) QQuickPointerDevice::PointerType::Finger QQuickPointerDevice::DeviceType::Mouse. The uninitiated reader of the code cannot see the additional type context when reading code and therefore someone might interpret Finger as a device type too. That's exactly what the existing unscoped enum policy tries to fix. Scoped enums provide this feature out of the box. Another example of a good naming policy (IMO) is QAbstractSocket (never mind the technical limitations). Here we could shorten the enum names using scoped enums: QAbstractSocket::SocketError::ConnectionRefusedError -> QAbstractSocket::SocketError::ConnectionRefused (or even QAbstractSocket::Error::ConnectionRefused) Name clash prevention was only ever secondary to readability. We don't need scoped enums to improve readability though, that doesn't seem like an argument. To take the finger/mouse example, we would use QQuickPointerDevice::FingerPointer or QQuickPointerDevice::PointerType_Finger and you get exactly the same benefit, no? Meanwhile, QQuickPointerDevice::DeviceType::Mouse is not improving readability IMO but adding noise. The name "PointerDevice" gives the exactly the context required to get an idea about the meaning of "Mouse". API is like documentation, there is a balance to be struck between conciseness and verbosity. If text is too verbose, it's hard to read. If text is too concise, then it may fail to convey crucial information. So we have three arguments for scoped enums: 1. Avoids clashes 2. Improves readability 3. Adds type safety After some of enum class use myself and seeing their use in code reviews, I feel that to avoid (1) and achieve (2) we don't need enum classes. After all we've solved this before with prefixing. I do buy a little into the type safety argument, but I feel that affects more the internal use (where we sometimes do things on a low-level and need it) in contrast to our API where we can make sure that the enums are used in good context. The risk I start to see with enum classes is that they present an opportunity where we can spend less time thinking about good names in the API. They present a tool that allow us to just use any name we like and not worry so much about the big picture of the class(es) they appear in. If we decide to allow deciding on a case-by-case basis, then I encourage everyone to carefully look at newly introduced enums, their values and their context in the upcoming "API review season". Simon 2. Scoped enums for the sake of type safety is a valid use case, but can be solved by promoting a warning to an error, and as such shouldn’t block us moving forward with #1. I don't see how a policy of elevating warnings to errors is better that using a language inbuilt error due to strong type safety. -- Alex ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
On Wed, Aug 15, 2018 at 07:32:48AM +, Alex Blasche wrote: > I don't think we have ever not permitted exceptions to official > policy. Therefore, take it for granted that the policy can be ignored > such as in the case presented by Allan. Having said that the default > should be the use of scoped enums even inside of classes. [...] More a general remark than a response to this particular message, or even thread. Still: One thing that severely annoys me, and has done so for a few years now, especially in leaf modules, is the piecemeal introduction of API style changes. While good old Qt API may look a bit unfashionable and outdated at times, it had the huge advantage of being consistent. The new styles usually do have some benefit, but their use starts by doing actual damage due to the introduced inconsistency. I'd rather stay consistent as long as reasonably possible, and then do big jumps, instead of seeing this GTK-style jumble all day. Andre' ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 15. Aug 2018, at 11:33, Tor Arne Vestbø wrote: > > > >> On 15 Aug 2018, at 09:32, Alex Blasche wrote: >> >> >> >>> -Original Message- >>> From: Tor Arne Vestbø >>> 1. Scoped enums (enum class) for the sake of avoiding name clashes is >>> useful for >>> global enums, but when the enum lives inside a class, the chance that we’ll >>> see a >>> naming clash is minor, and using scoped enums in that case arguably has >>> negative effects on code writability/readability. As a result, we should >>> update >>> the policy to not mandate scoped enums when the enum lives inside a class. >> >> I don't think we have ever not permitted exceptions to official policy. >> Therefore, take it for granted that the policy can be ignored such as in the >> case presented by Allan. Having said that the default should be the use of >> scoped enums even inside of classes. > > Fair enough, but having the policy means following that by default, resulting > in patches defaulting to the policy because it’s a policy, not taking each > individual case into consideration. Since I’m arguing that the default of > preferring scoped enums inside classes will in most cases result in less > writable/readable code, I don’t want us to default to something that produces > worse results than defaulting to the opposite. > >> I don't consider the longer names detrimental for writability and usability. >> Writability is easily solved with code completion and readability is >> actually better because the type adds additional context info. To cite an >> example from Shawn's patch: >> >> QQuickPointerDevice::Finger >> QQuickPointerDevice::Mouse >> >> is far less descriptive than (and not compliant to current naming convention) >> >> QQuickPointerDevice::PointerType::Finger >> QQuickPointerDevice::DeviceType::Mouse. >> > > When you write these enums in isolation, yes. > > But that’s not how the enums are used in actual code: > > if (eventPoint->state() != QQuickEventPoint::Released && > wantsEventPoint(eventPoint)) > > if (dev->type() == QQuickPointerDevice::TouchScreen) > > vs > > if (eventPoint->state() != QQuickEventPoint::State::Released && > wantsEventPoint(eventPoint)) > > if (dev->type() == QQuickPointerDevice::DeviceType::TouchScreen) > > The context is already there in most cases, which makes forcing the name of > the enum into the code redundant. > > And if the context is not there, you can still write > QQuickPointerDevice::DeviceType::TouchScreen to be more explicit _when_ > needed, even without scoped enums. > > Or just DeviceType::TouchScreen, if you are inside the class. Or even just > TouchScreen if the context is obvious from the rest of the call. > > Using scoped enums removes this flexibility. > >> The uninitiated reader of the code cannot see the additional type context >> when reading code and therefore someone might interpret Finger as a device >> type too. > > As argued above, the uninitiated reader _can_. > >> >>> 2. Scoped enums for the sake of type safety is a valid use case, but can be >>> solved >>> by promoting a warning to an error, and as such shouldn’t block us moving >>> forward with #1. >> >> I don't see how a policy of elevating warnings to errors is better that >> using a language inbuilt error due to strong type safety. > > Of course it’s not better, if you focus only on that. The use of a > warning-as-error instead of a language inbuilt is a workable practical > compromise because the language inbuilt comes with other downside, as > discussed above. Note that this warning does not cover all cases where conversions of enums to int are done where they where probably not intended. E.g. if (dev->type()) // forgot to add the actual comparison itemModel->sort(Qt::AscendingOrder); // forgot the column as the first argument Br, Eike -- Eike Ziller Principal Software Engineer The Qt Company GmbH Rudower Chaussee 13 D-12489 Berlin eike.zil...@qt.io http://qt.io Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 15 Aug 2018, at 09:32, Alex Blasche wrote: > > > >> -Original Message- >> From: Tor Arne Vestbø >> 1. Scoped enums (enum class) for the sake of avoiding name clashes is useful >> for >> global enums, but when the enum lives inside a class, the chance that we’ll >> see a >> naming clash is minor, and using scoped enums in that case arguably has >> negative effects on code writability/readability. As a result, we should >> update >> the policy to not mandate scoped enums when the enum lives inside a class. > > I don't think we have ever not permitted exceptions to official policy. > Therefore, take it for granted that the policy can be ignored such as in the > case presented by Allan. Having said that the default should be the use of > scoped enums even inside of classes. Fair enough, but having the policy means following that by default, resulting in patches defaulting to the policy because it’s a policy, not taking each individual case into consideration. Since I’m arguing that the default of preferring scoped enums inside classes will in most cases result in less writable/readable code, I don’t want us to default to something that produces worse results than defaulting to the opposite. > I don't consider the longer names detrimental for writability and usability. > Writability is easily solved with code completion and readability is actually > better because the type adds additional context info. To cite an example from > Shawn's patch: > > QQuickPointerDevice::Finger > QQuickPointerDevice::Mouse > > is far less descriptive than (and not compliant to current naming convention) > > QQuickPointerDevice::PointerType::Finger > QQuickPointerDevice::DeviceType::Mouse. > When you write these enums in isolation, yes. But that’s not how the enums are used in actual code: if (eventPoint->state() != QQuickEventPoint::Released && wantsEventPoint(eventPoint)) if (dev->type() == QQuickPointerDevice::TouchScreen) vs if (eventPoint->state() != QQuickEventPoint::State::Released && wantsEventPoint(eventPoint)) if (dev->type() == QQuickPointerDevice::DeviceType::TouchScreen) The context is already there in most cases, which makes forcing the name of the enum into the code redundant. And if the context is not there, you can still write QQuickPointerDevice::DeviceType::TouchScreen to be more explicit _when_ needed, even without scoped enums. Or just DeviceType::TouchScreen, if you are inside the class. Or even just TouchScreen if the context is obvious from the rest of the call. Using scoped enums removes this flexibility. > The uninitiated reader of the code cannot see the additional type context > when reading code and therefore someone might interpret Finger as a device > type too. As argued above, the uninitiated reader _can_. > >> 2. Scoped enums for the sake of type safety is a valid use case, but can be >> solved >> by promoting a warning to an error, and as such shouldn’t block us moving >> forward with #1. > > I don't see how a policy of elevating warnings to errors is better that using > a language inbuilt error due to strong type safety. Of course it’s not better, if you focus only on that. The use of a warning-as-error instead of a language inbuilt is a workable practical compromise because the language inbuilt comes with other downside, as discussed above. Tor Arne ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> -Original Message- > From: Tor Arne Vestbø > 1. Scoped enums (enum class) for the sake of avoiding name clashes is useful > for > global enums, but when the enum lives inside a class, the chance that we’ll > see a > naming clash is minor, and using scoped enums in that case arguably has > negative effects on code writability/readability. As a result, we should > update > the policy to not mandate scoped enums when the enum lives inside a class. I don't think we have ever not permitted exceptions to official policy. Therefore, take it for granted that the policy can be ignored such as in the case presented by Allan. Having said that the default should be the use of scoped enums even inside of classes. I don't consider the longer names detrimental for writability and usability. Writability is easily solved with code completion and readability is actually better because the type adds additional context info. To cite an example from Shawn's patch: QQuickPointerDevice::Finger QQuickPointerDevice::Mouse is far less descriptive than (and not compliant to current naming convention) QQuickPointerDevice::PointerType::Finger QQuickPointerDevice::DeviceType::Mouse. The uninitiated reader of the code cannot see the additional type context when reading code and therefore someone might interpret Finger as a device type too. That's exactly what the existing unscoped enum policy tries to fix. Scoped enums provide this feature out of the box. Another example of a good naming policy (IMO) is QAbstractSocket (never mind the technical limitations). Here we could shorten the enum names using scoped enums: QAbstractSocket::SocketError::ConnectionRefusedError -> QAbstractSocket::SocketError::ConnectionRefused (or even QAbstractSocket::Error::ConnectionRefused) Name clash prevention was only ever secondary to readability. > 2. Scoped enums for the sake of type safety is a valid use case, but can be > solved > by promoting a warning to an error, and as such shouldn’t block us moving > forward with #1. I don't see how a policy of elevating warnings to errors is better that using a language inbuilt error due to strong type safety. -- Alex ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 14 Aug 2018, at 15:29, Allan Sandfeld Jensen wrote: > > I agree, at least in most cases. There are still cases where I would prefer > unscoped enums. For instance in my recent QColorSpace patch, I ended up using > both types of enums under QColorSpace. The enum that represented specific > colorspaces that looked best without an extra scope I left unscoped (e.g. > QColorSpace::SRgb as opposed to QColorSpace::PredefinedColorSpace::SRgb), > enums that represented specific property types that couldn't be used alone to > construct a QColorSpace, I scoped (E.g. QColorSpace::Gamut::AdobeRgb). A policy not mandating use of scoped enums everywhere would still allow you to use them where it makes sense :) Tor Arne ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
I’ve lost track of what you are arguing for here 😊 To bring the discussion back, here are my main points: 1. Scoped enums (enum class) for the sake of avoiding name clashes is useful for global enums, but when the enum lives inside a class, the chance that we’ll see a naming clash is minor, and using scoped enums in that case arguably has negative effects on code writability/readability. As a result, we should update the policy to not mandate scoped enums when the enum lives inside a class. 2. Scoped enums for the sake of type safety is a valid use case, but can be solved by promoting a warning to an error, and as such shouldn’t block us moving forward with #1. Cheers, Tor Arne > On 14 Aug 2018, at 15:00, Eike Ziller wrote: > > > >> On 14. Aug 2018, at 13:18, Tor Arne Vestbø wrote: >> >> >>> On 14 Aug 2018, at 13:13, Eike Ziller wrote: >>> >>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2347.pdf states >>> the problems that were the driver for creating strongly typed enums: >>> >>> 1. Implicit conversion to integer >>> 2. Inability to specify underlying type >> >> https://en.cppreference.com/w/cpp/language/enum describes types for unscoped >> enums too, curious, what’s the difference for the scoped enums? >> >>> 3. Scoping >>> that specifically mentions the enum inside a class use case a primary driver? >>> >>> Why would the implicit conversion problem be any different for enums inside >>> a class? >> >> It wouldn’t. I was referring to the scoping/name clash, which is what this >> discussion has been largely focusing on (so far). > > Now you cut off context. I was answering to: > > > There is one more very important aspect. Scoped enums can have dedicated types and are type safe. This could have easily caught issues like (which coincidently was pointed out to me this morning): > > https://codereview.qt-project.org/#/c/236736/ > > And I believe that is a very good reason to still prefer scoped enums. How frequent is this class of bugs? >>> >>> Frequent enough for it being the primary driver behind the introduction of >>> scoped enums?!? >> >> Do you have a source for this, that specifically mentions the enum inside a >> class use case a primary driver? > > Which is about type-safety, not scoping. > > -- > Eike Ziller > Principal Software Engineer > > The Qt Company GmbH > Rudower Chaussee 13 > D-12489 Berlin > eike.zil...@qt.io > http://qt.io > Geschäftsführer: Mika Pälsi, > Juha Varelius, Mika Harjuaho > Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, > HRB 144331 B ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
On Dienstag, 14. August 2018 14:27:27 CEST Alex Blasche wrote: > > -Original Message- > > From: Tor Arne Vestbø > > > > > > To quote the policy: > > > > > > > > > > > > "By comparison the following example illustrates the dangers of missing > > > type > > > safety and giving general names to conventional enum values:" > > > > > ... > > > " One guideline for naming enum types is to repeat at least one element > > > of the > > > enum type name in each of the enum values" > > > > > > > > > > > I think the policy is explicit. > > > > > > The policy uses a global enum as an example. Obviously we need some sort > > of scoping there, either in the enum value names, or by using scoped > > enums. We agree there, and can leave that part of the discussion as > > solved. > > The discussion here is about what to do when the enum lives inside a > > class, which would already solve the naming clash issue in most cases. > > > This is not about namespace clashes or inside or outside classes. It is > about readability. Unscoped enums don’t require the enum type to be > mentioned although the type name tremendously helps as context info for > enum values. Without this context the enum is harder to understand. Hence a > more verbose name (repeating essential part of the type) is chosen. > And we have not even talked about strong typing of scoped versions (see > Eike's comment) > Scoped enums have a few advantages which are sufficient for me to accept its > use. Combine that with a naming policy that is close to what we had for > unscoped enums and you got a winner for me 😉 I agree, at least in most cases. There are still cases where I would prefer unscoped enums. For instance in my recent QColorSpace patch, I ended up using both types of enums under QColorSpace. The enum that represented specific colorspaces that looked best without an extra scope I left unscoped (e.g. QColorSpace::SRgb as opposed to QColorSpace::PredefinedColorSpace::SRgb), enums that represented specific property types that couldn't be used alone to construct a QColorSpace, I scoped (E.g. QColorSpace::Gamut::AdobeRgb). If we applied a policy like that to all of Qt, most enums would be scoped, but a few such as the special types of QJsonValue would not be. Best regards 'Allan ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 14. Aug 2018, at 13:18, Tor Arne Vestbø wrote: > > >> On 14 Aug 2018, at 13:13, Eike Ziller wrote: >> >> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2347.pdf states the >> problems that were the driver for creating strongly typed enums: >> >> 1. Implicit conversion to integer >> 2. Inability to specify underlying type > > https://en.cppreference.com/w/cpp/language/enum describes types for unscoped > enums too, curious, what’s the difference for the scoped enums? > >> 3. Scoping >> >>> that specifically mentions the enum inside a class use case a primary >>> driver? >> >> Why would the implicit conversion problem be any different for enums inside >> a class? > > It wouldn’t. I was referring to the scoping/name clash, which is what this > discussion has been largely focusing on (so far). Now you cut off context. I was answering to: There is one more very important aspect. Scoped enums can have dedicated >>> types and are type safe. This could have easily caught issues like (which >>> coincidently was pointed out to me this morning): https://codereview.qt-project.org/#/c/236736/ And I believe that is a very good reason to still prefer scoped enums. >>> >>> How frequent is this class of bugs? >> >> Frequent enough for it being the primary driver behind the introduction of >> scoped enums?!? > > Do you have a source for this, that specifically mentions the enum inside a > class use case a primary driver? Which is about type-safety, not scoping. -- Eike Ziller Principal Software Engineer The Qt Company GmbH Rudower Chaussee 13 D-12489 Berlin eike.zil...@qt.io http://qt.io Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> -Original Message- > From: Tor Arne Vestbø > > To quote the policy: > > > > "By comparison the following example illustrates the dangers of missing type > safety and giving general names to conventional enum values:" > > ... > > " One guideline for naming enum types is to repeat at least one element of > > the > enum type name in each of the enum values" > > > > I think the policy is explicit. > > The policy uses a global enum as an example. Obviously we need some sort of > scoping there, either in the enum value names, or by using scoped enums. We > agree there, and can leave that part of the discussion as solved. > > The discussion here is about what to do when the enum lives inside a class, > which would already solve the naming clash issue in most cases. This is not about namespace clashes or inside or outside classes. It is about readability. Unscoped enums don’t require the enum type to be mentioned although the type name tremendously helps as context info for enum values. Without this context the enum is harder to understand. Hence a more verbose name (repeating essential part of the type) is chosen. And we have not even talked about strong typing of scoped versions (see Eike's comment) Scoped enums have a few advantages which are sufficient for me to accept its use. Combine that with a naming policy that is close to what we had for unscoped enums and you got a winner for me 😉 -- Alex ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 14 Aug 2018, at 13:13, Eike Ziller wrote: > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2347.pdf states the > problems that were the driver for creating strongly typed enums: > > 1. Implicit conversion to integer > 2. Inability to specify underlying type https://en.cppreference.com/w/cpp/language/enum describes types for unscoped enums too, curious, what’s the difference for the scoped enums? > 3. Scoping > >> that specifically mentions the enum inside a class use case a primary driver? > > Why would the implicit conversion problem be any different for enums inside a > class? It wouldn’t. I was referring to the scoping/name clash, which is what this discussion has been largely focusing on (so far). Tor Arne ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 14. Aug 2018, at 12:30, Tor Arne Vestbø wrote: > >> >> On 14 Aug 2018, at 12:13, Alex Blasche wrote: >> >> >> >>> -Original Message- >>> From: Tor Arne Vestbø >>> That circular logic 😊 Or at least arguing that we should maintain the >>> policy not >>> because it makes sense, but just because we’ve done so in the past. >>> >>> You are not questioning _why_ we have done so in the past (for decades as >>> you >>> say), which should be the input to whether or not we want to maintain the >>> policy >>> as is, or update it to match the current landscape of Qt and C++. >>> >>> The mere fact that we’ve had a policy does not have any value. >> >> Well, so far you only questioned the scoped enum case and making an argument >> for the scoped enum case based on so far accepted rules for unscoped ones >> is valid. > > So let’s explicitly make this a discussion about the whole policy then, since > it seems that wasn’t obvious 😊 > >> >> Therefore QQuickEventPoint::State::Pressed is no more verbose than >>> QQuickEventPoint::PressedState. >>> >>> Again, the policy does not mandate that in it’s current form in my reading >>> of it, >>> and even if it did, that would be a bad policy and we should fix it, not >>> just blindly >>> accept it as set in stone, because we’ve followed it "for decades". >> >> To quote the policy: >> >> "By comparison the following example illustrates the dangers of missing type >> safety and giving general names to conventional enum values:" >> ... >> " One guideline for naming enum types is to repeat at least one element of >> the enum type name in each of the enum values" >> >> I think the policy is explicit. > > The policy uses a global enum as an example. Obviously we need some sort of > scoping there, either in the enum value names, or by using scoped enums. We > agree there, and can leave that part of the discussion as solved. > > The discussion here is about what to do when the enum lives inside a class, > which would already solve the naming clash issue in most cases. > >> Whether you think it is bad is a different kind of thing. Please don't mix >> that. > > That’s kind of the point of bringing up a policy on the mailing list, to > argue that it’s bad, and we should change it… > >> Personally, I think this naming rule has served us well and although it is >> not consistently applied the majority of enums does follow them. >> In summary, there is very little verbosity added even by existing rules. Mind >>> you, Qt has always favoured readability and accepted verbosity as a >>> consequence in times of code completion. >>> >>> Code completion (for those that use it), only helps with writing code, and >>> as we >>> all know, most time is spent reading code 😊 >>> There is one more very important aspect. Scoped enums can have dedicated >>> types and are type safe. This could have easily caught issues like (which >>> coincidently was pointed out to me this morning): https://codereview.qt-project.org/#/c/236736/ And I believe that is a very good reason to still prefer scoped enums. >>> >>> How frequent is this class of bugs? >> >> Frequent enough for it being the primary driver behind the introduction of >> scoped enums?!? > > Do you have a source for this, http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2347.pdf states the problems that were the driver for creating strongly typed enums: 1. Implicit conversion to integer 2. Inability to specify underlying type 3. Scoping > that specifically mentions the enum inside a class use case a primary driver? Why would the implicit conversion problem be any different for enums inside a class? > > You conveniently left out this part of my reply: > >>> Clang warns about this with -Wenum-compare for both unstopped and scoped >>> enums, we could easily make that and error and have the best of both. > > > Cheers, > Tor Arne > >> >> -- >> Alex >> >> ___ >> Development mailing list >> Development@qt-project.org >> http://lists.qt-project.org/mailman/listinfo/development > > ___ > Development mailing list > Development@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development -- Eike Ziller Principal Software Engineer The Qt Company GmbH Rudower Chaussee 13 D-12489 Berlin eike.zil...@qt.io http://qt.io Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 14 Aug 2018, at 12:13, Alex Blasche wrote: > > > >> -Original Message- >> From: Tor Arne Vestbø >> That circular logic 😊 Or at least arguing that we should maintain the policy >> not >> because it makes sense, but just because we’ve done so in the past. >> >> You are not questioning _why_ we have done so in the past (for decades as you >> say), which should be the input to whether or not we want to maintain the >> policy >> as is, or update it to match the current landscape of Qt and C++. >> >> The mere fact that we’ve had a policy does not have any value. > > Well, so far you only questioned the scoped enum case and making an argument > for the scoped enum case based on so far accepted rules for unscoped ones is > valid. So let’s explicitly make this a discussion about the whole policy then, since it seems that wasn’t obvious 😊 > > >>> Therefore QQuickEventPoint::State::Pressed is no more verbose than >> QQuickEventPoint::PressedState. >> >> Again, the policy does not mandate that in it’s current form in my reading >> of it, >> and even if it did, that would be a bad policy and we should fix it, not >> just blindly >> accept it as set in stone, because we’ve followed it "for decades". > > To quote the policy: > > "By comparison the following example illustrates the dangers of missing type > safety and giving general names to conventional enum values:" > ... > " One guideline for naming enum types is to repeat at least one element of > the enum type name in each of the enum values" > > I think the policy is explicit. The policy uses a global enum as an example. Obviously we need some sort of scoping there, either in the enum value names, or by using scoped enums. We agree there, and can leave that part of the discussion as solved. The discussion here is about what to do when the enum lives inside a class, which would already solve the naming clash issue in most cases. > Whether you think it is bad is a different kind of thing. Please don't mix > that. That’s kind of the point of bringing up a policy on the mailing list, to argue that it’s bad, and we should change it… > Personally, I think this naming rule has served us well and although it is > not consistently applied the majority of enums does follow them. > >>> In summary, there is very little verbosity added even by existing rules. >>> Mind >> you, Qt has always favoured readability and accepted verbosity as a >> consequence in times of code completion. >> >> Code completion (for those that use it), only helps with writing code, and >> as we >> all know, most time is spent reading code 😊 >> >>> There is one more very important aspect. Scoped enums can have dedicated >> types and are type safe. This could have easily caught issues like (which >> coincidently was pointed out to me this morning): >>> >>> https://codereview.qt-project.org/#/c/236736/ >>> >>> And I believe that is a very good reason to still prefer scoped enums. >> >> How frequent is this class of bugs? > > Frequent enough for it being the primary driver behind the introduction of > scoped enums?!? Do you have a source for this, that specifically mentions the enum inside a class use case a primary driver? You conveniently left out this part of my reply: >> Clang warns about this with -Wenum-compare for both unstopped and scoped >> enums, we could easily make that and error and have the best of both. Cheers, Tor Arne > > -- > Alex > > ___ > Development mailing list > Development@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> -Original Message- > From: Tor Arne Vestbø > That circular logic 😊 Or at least arguing that we should maintain the policy > not > because it makes sense, but just because we’ve done so in the past. > > You are not questioning _why_ we have done so in the past (for decades as you > say), which should be the input to whether or not we want to maintain the > policy > as is, or update it to match the current landscape of Qt and C++. > > The mere fact that we’ve had a policy does not have any value. Well, so far you only questioned the scoped enum case and making an argument for the scoped enum case based on so far accepted rules for unscoped ones is valid. > > Therefore QQuickEventPoint::State::Pressed is no more verbose than > QQuickEventPoint::PressedState. > > Again, the policy does not mandate that in it’s current form in my reading of > it, > and even if it did, that would be a bad policy and we should fix it, not just > blindly > accept it as set in stone, because we’ve followed it "for decades". To quote the policy: "By comparison the following example illustrates the dangers of missing type safety and giving general names to conventional enum values:" ... " One guideline for naming enum types is to repeat at least one element of the enum type name in each of the enum values" I think the policy is explicit. Whether you think it is bad is a different kind of thing. Please don't mix that. Personally, I think this naming rule has served us well and although it is not consistently applied the majority of enums does follow them. > > In summary, there is very little verbosity added even by existing rules. > > Mind > you, Qt has always favoured readability and accepted verbosity as a > consequence in times of code completion. > > Code completion (for those that use it), only helps with writing code, and as > we > all know, most time is spent reading code 😊 > > > There is one more very important aspect. Scoped enums can have dedicated > types and are type safe. This could have easily caught issues like (which > coincidently was pointed out to me this morning): > > > > https://codereview.qt-project.org/#/c/236736/ > > > > And I believe that is a very good reason to still prefer scoped enums. > > How frequent is this class of bugs? Frequent enough for it being the primary driver behind the introduction of scoped enums?!? Mind you even our own policies state this limitation of unscoped enums. -- Alex ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 14 Aug 2018, at 09:16, Alex Blasche wrote:
>
>> I
>> do not think that using class enums inside existing classes is a win for code
>> readability/writability:
>>
>> When you have
>>
>> switch (point->state()) {
>>
>> It's pretty obvious what case QQuickEventPoint::Pressed: refers to. being
>> overly
>> explicit with case QQuickEventPoint::State::Pressed: just adds more to type
>> for
>> no reason.
>
> Similar to Guiseppe, I would argue that the above enum does not comply with
> the convention for unscoped enums either.
I would disagree with that interpretation.
> The enum value would have to repeat at least some part of the enum type.
> That's a rule that has existed for decades now.
That circular logic 😊 Or at least arguing that we should maintain the policy
not because it makes sense, but just because we’ve done so in the past.
You are not questioning _why_ we have done so in the past (for decades as you
say), which should be the input to whether or not we want to maintain the
policy as is, or update it to match the current landscape of Qt and C++.
The mere fact that we’ve had a policy does not have any value.
> Therefore QQuickEventPoint::State::Pressed is no more verbose than
> QQuickEventPoint::PressedState.
Again, the policy does not mandate that in it’s current form in my reading of
it, and even if it did, that would be a bad policy and we should fix it, not
just blindly accept it as set in stone, because we’ve followed it "for decades".
> In summary, there is very little verbosity added even by existing rules. Mind
> you, Qt has always favoured readability and accepted verbosity as a
> consequence in times of code completion.
Code completion (for those that use it), only helps with writing code, and as
we all know, most time is spent reading code 😊
> There is one more very important aspect. Scoped enums can have dedicated
> types and are type safe. This could have easily caught issues like (which
> coincidently was pointed out to me this morning):
>
> https://codereview.qt-project.org/#/c/236736/
>
> And I believe that is a very good reason to still prefer scoped enums.
How frequent is this class of bugs?
Clang warns about this with -Wenum-compare for both unstopped and scoped enums,
we could easily make that and error and have the best of both.
Cheers,
Tor Arne
>
> --
> Alex
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> -Original Message-
> From: Tor Arne Vestbø
> Sent: Monday, 13 August 2018 16:40
> To: Alex Blasche
> Cc: development@qt-project.org; Simon Hausmann
> Subject: Re: [Development] Naming convention for (scoped) enums
>
> Bringing this up again in light of e.g. https://codereview.qt-
> project.org/#/c/235167/
>
> When I gave my support to this I thought we were talking about global enums.
I was certainly not limiting myself to those only. That's just a coincidence
that I chose a global enum for the policy.
> I
> do not think that using class enums inside existing classes is a win for code
> readability/writability:
>
> When you have
>
> switch (point->state()) {
>
> It's pretty obvious what case QQuickEventPoint::Pressed: refers to. being
> overly
> explicit with case QQuickEventPoint::State::Pressed: just adds more to type
> for
> no reason.
Similar to Guiseppe, I would argue that the above enum does not comply with the
convention for unscoped enums either. The enum value would have to repeat at
least some part of the enum type. That's a rule that has existed for decades
now. Therefore QQuickEventPoint::State::Pressed is no more verbose than
QQuickEventPoint::PressedState.
In summary, there is very little verbosity added even by existing rules. Mind
you, Qt has always favoured readability and accepted verbosity as a consequence
in times of code completion. There is one more very important aspect. Scoped
enums can have dedicated types and are type safe. This could have easily caught
issues like (which coincidently was pointed out to me this morning):
https://codereview.qt-project.org/#/c/236736/
And I believe that is a very good reason to still prefer scoped enums.
--
Alex
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
On Aug 13, 2018, at 18:12, Giuseppe D'Angelo via Development wrote: > Il 13/08/2018 16:40, Tor Arne Vestbø ha scritto: >> Or: >> if (event->device()->pointerType() != QQuickPointerDevice::Finger >> Gives me all the info I need, and having to type or read this instead is >> worse in my opinion: > > This is actually against the old "non-enum class" coding standards: one must > repeat the enumeration name in the enumerators. Then, the difference is > between something like > > QQuickPointerDevice::FingerPointerType (?) > > and > > QQuickPointerDevice::PointerType::Finger > > So quite minor, all in all… I would prefer the enum class over prefixing. But it works fine to do without both, too. It came up because I tried to add Scroll as a DeviceType, so there was a clash in that patch. But then I decided the reason I wanted that was not a great one anyway. So this change is not urgently needed now, it’s just a matter of doing what will make the most sense for the eventual supported API (when we get around to making QQuickPointerHandler and its subclasses public, which is not for 5.12, then users will be able to subclass it and handle events) and for future-proofing (prevent clashes later). >> if (event->device()->pointerType() != >> QQuickPointerDevice::PointerType::Finger && >> I think we should revisit this policy, and only use it when there’s actually >> a clash. > > Which is always "too late" if we're talking about public APIs, as they're set > in stone. For the record, the class is still private for now. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
Il 13/08/2018 16:40, Tor Arne Vestbø ha scritto:
>>> Or:
>>> if (event->device()->pointerType() != QQuickPointerDevice::Finger
>>> Gives me all the info I need, and having to type or read this
>>> instead is worse in my opinion:
On 13 Aug 2018, at 18:12, Giuseppe D'Angelo via Development
wrote:
>> This is actually against the old "non-enum class" coding standards:
>> one must repeat the enumeration name in the enumerators.
... and yet we have violations of that in public APIs.
e.g. QValidator::State, with values { Invalid, Intermediate, Acceptable }
Tor Arne Vestbø (13 August 2018 18:19)
> Just because the old standard was also promoting less
> readable/writable code doesn’t make it a good thing that we should
> keep 😊
> The coding standard was likely based on global enums, and somehow we
> didn’t account for enums inside classes. Even today
> https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values
> doesn’t mention enums inside classes, and the example only shows
> global enums.
and the rationale for the rule doesn't really need it applied except to
global enums (and, rarely, within a class if there's a clash).
>>> if (event->device()->pointerType() !=
>>> QQuickPointerDevice::PointerType::Finger &&
>>> I think we should revisit this policy, and only use it when there’s
>>> actually a clash.
>> Which is always "too late" if we're talking about public APIs, as
>> they're set in stone.
including the ones set in stone that violate the policy we've not been
rigorous abut following. Meanwhile, when it comes to adding *new*
enums, do we really want to continue following this rule ?
> Of course. I’m just arguing that we shouldn’t continue down this road
> now that we’ve learned that the coding standard produces worse results
> for the specific case of enums inside classes.
+1
>> In the meanwhile, I would not work around it -- we need *some* enum
>> scoping anyhow, and enum classes are the simplest way possible to
>> not forget about it.
> Enum scoping is already achieved by the enum living inside a
> class. You already have your goal met 😊 If it’s a global enum, sure,
> make it a class enum instead of
> QQuickPointerDevice::FingerPointerType. And if it’s inside a class,
> and clashes, sure, make it a class enum. But for the common case,
> let’s prioritise readability/writability.
... and avoid pointless duplication.
Yes please,
Eddy.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
On 13 Aug 2018, at 18:12, Giuseppe D'Angelo via Development wrote: > > Il 13/08/2018 16:40, Tor Arne Vestbø ha scritto: >> Or: >> if (event->device()->pointerType() != QQuickPointerDevice::Finger >> Gives me all the info I need, and having to type or read this instead is >> worse in my opinion: > > This is actually against the old "non-enum class" coding standards: one must > repeat the enumeration name in the enumerators. Just because the old standard was also promoting less readable/writable code doesn’t make it a good thing that we should keep 😊 The coding standard was likely based on global enums, and somehow we didn’t account for enums inside classes. Even today https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values doesn’t mention enums inside classes, and the example only shows global enums. >> if (event->device()->pointerType() != >> QQuickPointerDevice::PointerType::Finger && >> I think we should revisit this policy, and only use it when there’s actually >> a clash. > > Which is always "too late" if we're talking about public APIs, as they're set > in stone. Of course. I’m just arguing that we shouldn’t continue down this road now that we’ve learned that the coding standard produces worse results for the specific case of enums inside classes. > In the meanwhile, I would not work around it -- we need *some* enum scoping > anyhow, and enum classes are the simplest way possible to not forget about it. Enum scoping is already achieved by the enum living inside a class. You already have your goal met 😊 If it’s a global enum, sure, make it a class enum instead of QQuickPointerDevice::FingerPointerType. And if it’s inside a class, and clashes, sure, make it a class enum. But for the common case, let’s prioritise readability/writability. Tor Arne ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
Il 13/08/2018 16:40, Tor Arne Vestbø ha scritto: Or: if (event->device()->pointerType() != QQuickPointerDevice::Finger Gives me all the info I need, and having to type or read this instead is worse in my opinion: This is actually against the old "non-enum class" coding standards: one must repeat the enumeration name in the enumerators. Then, the difference is between something like QQuickPointerDevice::FingerPointerType (?) and QQuickPointerDevice::PointerType::Finger So quite minor, all in all... if (event->device()->pointerType() != QQuickPointerDevice::PointerType::Finger && I think we should revisit this policy, and only use it when there’s actually a clash. Which is always "too late" if we're talking about public APIs, as they're set in stone. By the way, the C++ community has already recognized the excessive verbosity of enum classes, and there are proposals such as http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1099r0.html that would drastically simplify the syntax. In the meanwhile, I would not work around it -- we need *some* enum scoping anyhow, and enum classes are the simplest way possible to not forget about it. My 2 c, -- Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer KDAB (France) S.A.S., a KDAB Group company Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com KDAB - The Qt, C++ and OpenGL Experts smime.p7s Description: Firma crittografica S/MIME ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
Bringing this up again in light of e.g.
https://codereview.qt-project.org/#/c/235167/
When I gave my support to this I thought we were talking about global enums. I
do not think that using class enums inside existing classes is a win for code
readability/writability:
When you have
switch (point->state()) {
It's pretty obvious what case QQuickEventPoint::Pressed: refers to. being
overly explicit with case QQuickEventPoint::State::Pressed: just adds more to
type for no reason.
Or:
if (event->device()->pointerType() != QQuickPointerDevice::Finger
Gives me all the info I need, and having to type or read this instead is worse
in my opinion:
if (event->device()->pointerType() !=
QQuickPointerDevice::PointerType::Finger &&
I think we should revisit this policy, and only use it when there’s actually a
clash.
Tor Arne
> On 22 May 2018, at 10:04, Alex Blasche wrote:
>
> I updated the enum section:
>
> https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values
>
> --
> Alex
>
>
> From: Development
> on behalf of
> Lars Knoll
> Sent: Tuesday, 22 May 2018 9:30:18 AM
> To: Christian Kandeler
> Cc: Qt development mailing list
> Subject: Re: [Development] Naming convention for (scoped) enums
>
>
>
>> On 17 May 2018, at 11:35, Christian Kandeler
>> wrote:
>>
>> On Thu, 17 May 2018 08:14:15 +
>> Alex Blasche wrote:
>>
>>> The naming conventions for enums state that each enum value name must
>>> repeat a part of the enum Type name (for details see
>>> https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values)
>>>
>>> In case of scoped enums this becomes a superfluous rule as the type has to
>>> be mentioned anyway. Does anybody object to modifying the above definition
>>> by adding an exception for scoped enums where you do not have to repeat a
>>> part of the enum type name?
>>
>> I would not have even assumed that the rule applies to scoped enums, but it
>> can't hurt to write it down explicitly. Perhaps the section should be
>> rewritten so that the unscoped enums are the special case rather than the
>> other way around.
>
> Agree. The default for new enums should be scoped enums.
>
> Cheers,
> Lars
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
I updated the enum section: https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values -- Alex From: Development on behalf of Lars Knoll Sent: Tuesday, 22 May 2018 9:30:18 AM To: Christian Kandeler Cc: Qt development mailing list Subject: Re: [Development] Naming convention for (scoped) enums > On 17 May 2018, at 11:35, Christian Kandeler wrote: > > On Thu, 17 May 2018 08:14:15 + > Alex Blasche wrote: > >> The naming conventions for enums state that each enum value name must repeat >> a part of the enum Type name (for details see >> https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values) >> >> In case of scoped enums this becomes a superfluous rule as the type has to >> be mentioned anyway. Does anybody object to modifying the above definition >> by adding an exception for scoped enums where you do not have to repeat a >> part of the enum type name? > > I would not have even assumed that the rule applies to scoped enums, but it > can't hurt to write it down explicitly. Perhaps the section should be > rewritten so that the unscoped enums are the special case rather than the > other way around. Agree. The default for new enums should be scoped enums. Cheers, Lars ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 17 May 2018, at 11:35, Christian Kandeler wrote: > > On Thu, 17 May 2018 08:14:15 + > Alex Blasche wrote: > >> The naming conventions for enums state that each enum value name must repeat >> a part of the enum Type name (for details see >> https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values) >> >> In case of scoped enums this becomes a superfluous rule as the type has to >> be mentioned anyway. Does anybody object to modifying the above definition >> by adding an exception for scoped enums where you do not have to repeat a >> part of the enum type name? > > I would not have even assumed that the rule applies to scoped enums, but it > can't hurt to write it down explicitly. Perhaps the section should be > rewritten so that the unscoped enums are the special case rather than the > other way around. Agree. The default for new enums should be scoped enums. Cheers, Lars ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
On Thu, 17 May 2018 08:14:15 + Alex Blasche wrote: > The naming conventions for enums state that each enum value name must repeat > a part of the enum Type name (for details see > https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values) > > In case of scoped enums this becomes a superfluous rule as the type has to be > mentioned anyway. Does anybody object to modifying the above definition by > adding an exception for scoped enums where you do not have to repeat a part > of the enum type name? I would not have even assumed that the rule applies to scoped enums, but it can't hurt to write it down explicitly. Perhaps the section should be rewritten so that the unscoped enums are the special case rather than the other way around. Christian ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
> On 17 May 2018, at 10:14, Alex Blasche wrote: > > Hi, > > The naming conventions for enums state that each enum value name must repeat > a part of the enum Type name (for details see > https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values) > > In case of scoped enums this becomes a superfluous rule as the type has to be > mentioned anyway. Does anybody object to modifying the above definition by > adding an exception for scoped enums where you do not have to repeat a part > of the enum type name? Sounds like a sensible thing +1 Tor Arne > -- > Alex > ___ > Development mailing list > Development@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Naming convention for (scoped) enums
Il 17/05/2018 10:14, Alex Blasche ha scritto: In case of scoped enums this becomes a superfluous rule as the type has to be mentioned anyway. Does anybody object to modifying the above definition by adding an exception for scoped enums where you do not have to repeat a part of the enum type name? I approve of the change. In Qt 5.11 we've already started introducing enum classes that do not repeat the enumeration name in the enumerators (e.g. QAbstractItemModel::CheckIndexOption). I would also double-down and say that every new enumeration in Qt has to be an enum class. My 2 cents, -- Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer KDAB (France) S.A.S., a KDAB Group company Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com KDAB - The Qt, C++ and OpenGL Experts smime.p7s Description: Firma crittografica S/MIME ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
[Development] Naming convention for (scoped) enums
Hi, The naming conventions for enums state that each enum value name must repeat a part of the enum Type name (for details see https://wiki.qt.io/API_Design_Principles#Naming_Enum_Types_and_Values) In case of scoped enums this becomes a superfluous rule as the type has to be mentioned anyway. Does anybody object to modifying the above definition by adding an exception for scoped enums where you do not have to repeat a part of the enum type name? -- Alex ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
