> On 31. Aug 2018, at 11:56, Tor Arne Vestbø <tor.arne.ves...@qt.io> 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) >> <gerrit-nore...@qt-project.org> 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
