Easier Inheritance
Closed this issue · 7 comments
ATF is a great package, and I'm really stoked it's now open for everyone to use.
I've found that a lot of the classes I've been using so far (PropertyGrid, PropertyEditor, PropertyGridView, etc) use restrictive access modifiers. I'd like to suggest "internal" be removed from codebase, and "private" be used minimally in favor of protected. These access modifiers make it difficult to re-use existing code and either force a lot of copy/paste or unnecessary forking.
Hello,
Thank you for the kind words and I'm glad if ATF is useful to you.
You bring up a very good point. The request to expose private members is very common. We've even been asked before to make literally every private and internal member be either protected or public.
The problem for us is that once a method or property or class or (rarely) a field is public, then it is frozen and we are constrained by it when trying to add new features or to fix problems, because we are very reluctant to make breaking changes. We are free to change private members. So, we tend to err on the side of keeping things private because we can always make it public in the future, but that's a one-way conversion and we can essentially never make a public member be private again or change it.
We try to imagine all the ways that a class might need to be customized and we make an effort to add customization points (events and protected virtual methods, for example) but we can't anticipate all possible uses.
Would you be willing to list specific members that you would like to see be made protected or public? This is normally an easy change for us to do, and then you won't have to duplicate the code.
Thank you!
--Ron
PropertyGridView's SelectProperty() I'd vote be made not internal.
I understand your argument regarding locking yourself in when making things more visible than private. I agree completely, especially with code deeper and deeper in the core of ATF. There are certain classes though that I'd expect to be more often inherited from than others like controls or services that are often more specific for tools. In those cases I'd rather see the properties protected instead of private, and if large scale changes are needed branch to a new class that you replace the old one with.
If users have to copy/paste to make changes to an existing control to make any changes then any concern about changing things out from underneath them is moot anyway.
Thanks for being so open to discussion.
- Colin
Good suggestion, Colin. Thank you. I made that one public. Do you have any other members that you think should be public?
--Ron
In PropertyGrid.cs DescriptionControl and m_descriptionTextBox being private made it hard to make changes to the PropertyGrid class's PropertyGridView. I basically had to copy-paste the whole PropertyGrid to change a couple small behaviors in the PropertyGridView.
Did you need access to the DescriptionControl's TextBrush, for a customized appearance? I could expose that. Or did you want to use your own bold-faced font? The description text can already be set. I don't see anything else in that private class that is worth customizing.
Looking back at the changes I made I think I was just trying running into the fact that m_propertyGridView is just so core to how the PropertyGrid class works, it's touched everywhere, and the m_descriptionTextBox was just one of the first things I ran into where there was a direct interaction. I guess scratch my last remark.
PropertyGridView.Pick might be another good candidate.