> On 15. Aug 2018, at 11:33, Tor Arne Vestbø <tor.arne.ves...@qt.io> wrote: > > > >> 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.
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
