> On 15 Aug 2018, at 09:32, Alex Blasche <alexander.blas...@qt.io> 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

Reply via email to