The parameter "SingleSolidRule" can be set to an arbitrary value different to 0 or 1. Thus, a static cast to this enum type is unspecified or even undefined behaviour: https://wiki.sei.cmu.edu/confluence/display/cplusplus/INT50-CPP.+Do+not+cast+to+an+out-of-range+enumeration+value

Since the enum has only two values a simple boolean will be enough. Alternatively, defining two constant long int variables instead will work, too. The variables don't need to reside in the header file but can go to the source.
And singleSolidRuleMode() should be declared as private function.

But I wonder whether it's a good idea to make this behaviour dependent on a user parameter because it's clearly a code smell (the parameter adds global mutable state, direct dependencies to other classes is added, writing test code will be more complicated). And there is a practicable problem if a user has changed the default parameter and shares a project with another user who uses the default value. After a recompute the project will break if a feature has multiple solids.

Using a hidden PropertyBool instead solves these problems. IMO, it makes most sense to add this property to the class Body or BodyBase and not to PartDesign::Feature.

Since the enum has only two values a simple boolean will be enough.

As said in the PR description - It is in preparation for additional mode that will allow multiple solids in body but warn user about creating such potentially unwanted situation. That's why it is enum value. I also personally dislike booleans as they are not very descriptive.

But I wonder whether it's a good idea to make this behaviour dependent on a user parameter because it's clearly a code smell (the parameter adds global mutable state, direct dependencies to other classes is added, writing test code will be more complicated). And there is a practicable problem if a user has changed the default parameter and shares a project with another user who uses the default value. After a recompute the project will break if a feature has multiple solids.

Yeah, I also thought about that. I'd still leave this option in the preferences as the default value used for new bodies but I can move this to the Body parameter. Then in fact it can be simple Boolean and the option warning users on this potentially unwanted scenario could be implemented globally.

What do you think?

What do you think?

I prefer adding a hidden property to the Body (or BodyBase) class. But we can bring this back to the forum and wait for opinions of the user base.

Yes I was referring to adding it to the Body class

Yeah, I also thought about that. I'd still leave this option in the preferences as the default value used for new bodies but I can move this to the Body parameter. Then in fact it can be simple Boolean and the option warning users on this potentially unwanted scenario could be implemented globally.

this seems like a better idea

@wwmayer I updated the code. I did implement it using normal (not hidden) PropertyBoolean on PartDesign::Body . I don't want to make it hidden as this would break the first of Nielsen’s Heuristics - Visibility of system status - causing possible confusion for users.

I don't want to make it hidden as this would break the first of Nielsen’s Heuristics - Visibility of system status - causing possible confusion for users.

No problem!

I only thought to make it hidden because it's considered as experimental at the moment and once it's considered stable I would have made it a regular property.

Yes, making it in the experimental group should be enough of a warning. At least I hope so!