I was quite surprised when I found that the BCheckboxControl does not feature a boolean variant of the SetValue method, which is quite awkward.
I suppose this is strictly consistent with class inheritance but shouldn’t a specialised control be allowed to have a native, more intuitive variant of SetValue (and GetValue while we are at it)?
(This almost reminds me of SAP where booleans are stored with X or an empty String,-)
All BControls have an int32 value, and you can use B_CONTROL_ON and B_CONTROL_OFF for the ones that can be toggled between only two values (such as checkboxes).
Maybe we could add an extra method, but it seems to add complexity for no real reason. It’s easy enough to convert between a bool and an int. In fact, SetValue(true) or SetValue(false) will compile just fine (there is some code doing this in Haiku sources with checkboxes).
If we added an extra method, these calls would become possibly ambiguous (did you mean to call the bool method, or cast the bool to an integer? both are valid C++ code), and so the compiler may error because it doesn’t know what you meant. So, an extra method would make things more complicated, and not solve any problem, I think?
Yes I understand that and used the constants, was just wondering why but it makes sense, let’s keep the API clean, Be engineers certainly knew what they were doing.
I wouldn’t rely on the conversion from bool to the constant values though, that will only work as long as the values of B_CONTROL_ON/OFF are infact 1 and 0, respectively.
In that sense this shortcut probably shouldn’t be used in Haiku itself…
I brought this up in the devel mailing list before. BControl has an int32 value. I brought up the idea of making a BTypedControl template with a passed in type e.g. bool in your case but the other devs didn’t like that idea. BColorControl has an rgb_color SetValue() override but it packs the color data into an int32 to call SetValue() so this is a design issue with other BControl inherited classes. Another option suggested is to set the value using a BMessage parameter and then you could AddBool or AddColor or whatever type you want that BMessage supports or SetData to do your own thing. This is all R2 type stuff because we’re stuck with the current BControl design however we could hack in a solution to BControl sooner as long as we don’t change SetValue(int32).
we could do the same for BCheckboxControl though, adding a delegate method SetBool(bool flag) that calls SetValue() with B_CONTROL_ON and B_CONTROL_OFF, resp.
We could add a SetValue(bool) to checkbox and radio button but that would trigger ambiguous value warnings SetValue(1) is that an int32 or a bool? So we’d have to add a SetBoolValue() method instead to avoid that. While that could be useful it covers the real issue that the value type is an int32 while it should be a bool. I’m not sure how worthwhile it would be to do this. The Value() would still be an int32 so then we’d have to add a BoolValue() method to get the value back out as a bool type. It’s not a generic solution that solves the root issue and I’m not sure it really helps that much in this case.
For BCheckBox and BRadioButton using an int32 is ok we are using a subset of possible values, and that’s good enough. The real problem here is something like BDecimalSpinner where the value is a double and BControl falls down because not all values can be represented by an int32. With BCheckbox we can get by with an int32 value and a comparison with B_CONTROL_OFF/ON. It’s not great but it is the design we inherited from Be.
And if it was something like SetState(bool) ?
Same idea with SetState(bool) vs. SetBool(bool) it is of limited value and doesn’t address the underlying issue. Ok perhaps your method name suggestion is a bit better than mine. This could be a helpful convenience but the burden is still on the programmer to understand what is going on behind the scenes (value is an int32).
I’m not totally against the idea but I’d prefer not to solve this problem in this way and wait until a generic solution is created to the underlying problem/design.
As far as I remember how C++ classes work, the “subclass” should make the original method private, so it can’t be called by outside code. Then the new method can be added as public method without causing ambiguity.
This should also fix cases where the bit width of the new type is more than 32, as the subclass itself will store the value as own member variable, and the variable in parent class is hidden and no longer updated. Obviously any remaining methods accessing the original variable also need to be overridden.
That’s why overriding SetValue() is not really a good idea. However, I see no reason why it shouldn’t have SetSelected()/IsSelected() methods like BMenuItem.
Why not SetToggle(bool)?
Some more modern controls can have a value AND a toggle (often used to expand or disable the control).
The meaning of toggle would vary by control.
For radios and checkboxes the toggle would also change the value between 0&1. For many other controls it would do nothing. For other controls it could toggle between expanded and unexpanded.