Use 'protected' and not 'private' in core and libraries

[bombasticbob]

Currently, with widespread use of 'private' in all of the core classes and libraries, it can be extremely difficult to make minor changes to a class implementation without (literally) copying the entire class implementation (and possibly all of the utility classes), re-naming everything, and then applying your changes to the new code. Wouldn't it be much SIMPLER if you could DERIVE your own class implementation (maybe override specific functions, or provide NEW ones that are application specfic) and access all of the (currently 'private') members with your 'specialized' or 'improved' version in order to implement it?

I had to make changes to HardwareSerial to improve 'send' performance at 115kbaud (it was only running at around 8k/sec instead of >10k/sec with an 8mhz clock, and I got it up to 9.7k/sec with my mods). I had to make copies of the appropriate core files and change the class name, and THEN place them in their own directory in 'sketchbook/libraries' and change the '#include' line to use my version of HardwareSerial. HOWEVER, it would have been MUCH easier if I could have simply DERIVED my own class, something like:

class MyHardwareSerial : public HardwareSerial

and then declared 'MySerial' as an external of type 'MyHardwareSerial' and used that instead of 'Serial'. For this to have worked at all, you would need to declare the 'private' members as 'protected' in HardwareSerial. That is the point.

Please make it easier to derive our own class implementations from the core classes and the supplied libraries. Please use 'protected' instead of 'private'. You can specifically tweek out the libraries for specific hardware that way, or perhaps combine the functionality of 2 classes into a single function to greatly improve performance. THIS is the kind of flexibility open source is supposed to provide. Using 'private' for class members GETS IN THE WAY of end-user flexibility.

[cmaglie]

A protected field means that we allow the use of that fields from subclasses. In other words, it becomes part of the public API and can not be changed any more without breaking compatibility with the subclasses that use it.

In particular, HardwareSerial has been improved a lot during the last year and most of the improvements involved a substantial change in the private fields. See #1711 for example, but if you search HardwareSerial you can find many more. if current methods were protected instead of private all these improvements would not have been possible or they would be possible at the cost of breaking all the existing subclasses (including yours maybe).

For these reasons IMHO private is not "an outdated and short-sighted concept", but blindly changing all private fields/method to protected is.

http://programmers.stackexchange.com/a/143739
http://stackoverflow.com/a/8353371/1655275

[bombasticbob]

you also force everyone to basically write their own derived classes by copying yours JUST to access something. Tying people's hands isn't very "open" now is it?

you seem to want to claim too much 'ownership' and control. I'm inclined to wildcard edit everything after installing anyway, then mark the code with "you will need to change 'private' to 'protected' in these files x.h y.h z.h" etc.. I've had several reasons for doing that (re-implementing hardware serial was one of them, so I wonder if my speedups were anything like the ones you guys made). but I have, on more than one occasion, "cloned" a library as "MySomething" with (naturally) 'protected' members, so that I could create derived classes and do direct access of things that need it with specialzed functionality.

If your data members were 'protected' instead of 'private', I would NOT! HAVE! TO! GO! THROUGH! THE! HASSLE! OF! CLONING! THE! ENTIRE! LIBRARY! (then making sure NOTHING ELSE REFERENCES the OLD one so my binary image doesn't bloat).

But consider this: if someone derives a class, and it breaks because you change something, then the end user will simply have to fix his code. Simple enough.

[matthijskooijman]

But consider this: if someone derives a class, and it breaks because you change something, then the end user will simply have to fix his code. Simple enough.

That's certainly a possibility, but right now apparently the policy is that "protected" means "part of stable API", hence it is used with caution. Advantages of dropping this policy and just making everything protected:

• More power for users, less copying of code.
• Less burden on Arduino developers to keep compatibility of internal fields and methods

The downside is of course that derived code breaks more often, but I guess that's probably a smaller burden than not being able to do things. Especially since I don't think that the "protected API" is often a well-thought-out API - I haven't actually seen any PR so far where the protected API was considered.

Overall, I'd be in favor of dropping the protected API concept and just making everything protected.

[cmaglie]

you also force everyone to basically write their own derived classes by copying yours JUST to access something. Tying people's hands isn't very "open" now is it? you seem to want to claim too much 'ownership' and control

Tens of authors have contributed to the HardwareSerial class and the code is open source and available for free: saying that they are trying to tie your hands and to "claims ownership and control" is really too much.

If your data members were 'protected' instead of 'private', I would NOT! HAVE! TO! GO! THROUGH! THE! HASSLE! OF! CLONING! THE! ENTIRE! LIBRARY!

I had already realized your point, thanks :-)

Just to be clear, blaming us of being "closed" or "screaming" with big fonts and lot of exclamation marks didn't support your arguments but, conversely, makes me want to ignore and to pass up this thread.

Now going back in topic:

But consider this: if someone derives a class, and it breaks because you change something, then the end user will simply have to fix his code. Simple enough.

No, it's not so simple. Maybe it's not a problem for you, but for a lot of users is.
When you release a public API you should keep it stable as much as possible, no matter how painful this is. Arduino has millions of users (yes, millions, if we consider derivatives and clones) and every time we make an incompatible change thousand of projects breaks.

Said that, let me show you an example:

99f2a27

In this commit the fields of Stream class related to timeout functionality have been changed from private to protected. Note that there is a compelling reason behind this change: it allows subclasses of Stream to optimize block reads with timeouts.

Now that those fields are protected do you think that we can freely change, for whatever reason, the Stream's timeout implementation in an incompatible way? No, we can't do it any more, unless there is a very strong reason to do so, because every subclass that depends on the current implementation will break. There are a lot of subclasses of Stream (basically everything that implements a bidirectional communication) and you can't say in advance how many of them use the _timeout field and how many projects will be affected.

That's certainly a possibility, but right now apparently the policy is that "protected" means "part of stable API", hence it is used with caution. Advantages of dropping this policy and just making everything protected:

We didn't have a "policy" that can be dropped here, when you define something protected you're implicitly saying that those fields are available for use in user's subclasses, this is implicit and enforced in the language. Once users starts to rely on them, you cannot change them any more. In other words they becomes part of your public API.

Look at the Stream example above: don't we have actually provided an API to handle Stream timeouts?

I'm not saying that it's always wrong to change something from private to protected, but doing it blindly with search&replace is wrong and every case needs a specific discussion.

[bombasticbob]

your point about Stream-based classes is definitely a good one, and it's the point I'm trying to make (so 'good start', still more to go).

I believe it would be MUCH better to fail 'open' than 'shut', marking things private as an exception rather than the rule, if you must use 'private' at all. The problem is that I see is a lot of (unnecessary) 'private' members when 'protected' would do. So unless you need private for a specific valid reason, can't you at least change everything ELSE to 'protected'? Do it for the developers that are trying to squeeze a bit more 'something' out of an AVR processor, and need to tweek things to work better [example as to why Stream has 'protected' members now, so other classes can optimize for performance].

Anyway, looks like I'll just have to continue cloning the libraries and changing the names if I want to add functionality or tweek something. I was hoping to change minds. Looks like it's not working.