Opened 14 years ago

Closed 9 years ago

#284 closed request (wontfix)

hide your members

Reported by: Peter Owned by: Markus Ringnér
Priority: major Milestone:
Component: classifier Version:
Keywords: Cc:

Description

We all know that members should not be public. However, in a few places we have protected members. I think we should move this to be private and add functions when needed.

I place this in classifier, but this ugly thing might be present also elsewhere.

Change History (13)

comment:1 Changed 14 years ago by Markus Ringnér

But these functions should then be protected rather than public and then one is in the define two interfaces well for a class scenario: the public and the protected. This is an issue that is hotly debated.

So I am not sure I agree. See for example item 19.8 in the C++-lite-faq:

http://home.wanadoo.nl/efx/c++-faq/basics-of-inheritance.html#faq-19.8

comment:2 Changed 14 years ago by Jari Häkkinen

Well, I do not like protected members ... what is the overhead to follow Peters suggestion and ignoring the FAQ? And I am a complex man, I know how to prioritize.

comment:3 Changed 14 years ago by Markus Ringnér

Well I guess what we end up with is that we produce a lot of protected functions that only set and get private members. In which case the protected interfaces will instead of behaving like a C struct (protected members) will be like a C struct with an interface. In which case things get slower and even if you may feel you have made a robust design it is not more robust in practice and our protected interface will look like C programmers thinking they are programming object oriented.

However, my point is not to argue one way or the other. But to argue against having a general rule for this. I think, if there is a case for a nice protected interface that makes the derived classes less dependent on the base class then surely one should not have protected members, but if there is not such a case, protected members will be as robust as a get/set interface. Afterall, we know that a class with a public interface only full of get/set functions indicates a bad design.

And I am a complex man, I know how to prioritize.

So you prioritize writing protected functions ahead of what?

comment:4 Changed 14 years ago by Peter

I am surprised, this ticket could start a debate.

Ever since you guys tried to teach me how to compile, you emphasized to have no public members referring to S. Meyers. I think you need to read his item again. His argument for having functions is valid also for the protected interface.

Herb Sutter is pretty clear about this issue. Only time members should not be private is when having C-like structs (e.g. std::pair), and you will never have C-like structs in a inheritance hierarchy, will you?

comment:5 Changed 14 years ago by Markus Ringnér

Fantastic! You write exactly my argument but come to the opposite conclusion!

According to what you write above they should indeed be protected members in most cases. Most often a base class is a C-like struct to derived classes (but of course not to users of the public interface). To see this try to think of a derived class not as a client of the base class but as an implementation of it (especially if the base class is an interface). Again, of course there are inheritance structures where protected interfaces and hiding members from derived classes makes sense and things more robust.

So, I agree with what you claim to have read in some Herb Sutter book, but disagree with your interpretation of it. No wonder this started a debate: in my view you took a rule from Sutter and wanted to strictly enforce it on a case which is listed by Sutter as an exception to the rule. I guess this issue is what I disguised as "our protected interface will look like C programmers thinking they are programming object oriented" above.

And I guess then that Sutter agrees with the author, Marshall Cline, of C++-faq who wrote in the item I quoted above:

'Whenever someone says to you, "You should always make data private," stop right there — it's an "always" or "never" rule, and those rules are what I call one-size-fits-all rules. The real world isn't that simple.'

It is intriguing to note that Marshall Cline was a peer-reviewer for the Sutter and Alexandrescu, C++ Coding Standards book and that Marshall Cline thinks it is the best book available on C++ coding standards.

comment:6 Changed 14 years ago by Peter

Wow, you not only interpret my words, but also manage to interpret what I read. And it all fits to your picture.

What Sutter (and Alexandrescu) wrote was that only time it is motivated to have public members are when you have C-like struct, in other words, a class with no functions. And when they got the discussion on protected they just concluded that the same arguments apply with the exception that you won't have a base class with no functions.

comment:7 Changed 14 years ago by Peter

Ok, let's sober up and be specific rather than general.

I checked in yat and we have protected members in Kernel, SupervisedClassifier?, FeatureSelection?, Sampler, Discrete, Continuous, OneDimensional?, OneDimensionalWeighted?, Score, NNI.

In some cases I could agree with Markus points that it is convenient and preferable to have them as protected (e.g. Sampler) but in other cases, e.g. FeatureSelector?, members should not be touched by daughter classes but only read and it is stupid to have them as protected.

I think we can agree on that we need to go through these classes and change them when it sensible. I guess it is harder to agree on what is sensible. But if start with the obvious cases... the rest will follow(?)

comment:8 in reply to:  6 ; Changed 14 years ago by Markus Ringnér

Replying to peter:

Wow, you not only interpret my words, but also manage to interpret what I read. And it all fits to your picture.

Well you wrote "Only time members should not be private is when having C-like structs (e.g. std::pair), and you will never have C-like structs in a inheritance hierarchy, will you?" in the comment after the one where I tried to make the point that "protected interfaces will instead of behaving like a C struct (protected members) will be like a C struct with an interface". So I found it amusing.

What Sutter (and Alexandrescu) wrote was that only time it is motivated to have public members are when you have C-like struct, in other words, a class with no functions. And when they got the discussion on protected they just concluded that the same arguments apply with the exception that you won't have a base class with no functions.

And I am arguing that the public interface and the protected interface are separate entities with separate design requirements and separate clients. "It is motivated to have public members if there are no public functions in the class" should in my opinion translate into "it is motivated to have protected members if there are no protected members in a the base class" and not into "if there are no public members then make the protected interface a function-based interface too".

comment:9 in reply to:  8 Changed 14 years ago by Markus Ringnér

Replying to markus:

"it is motivated to have protected members if there are no protected members in a the base class"

should be

"it is motivated to have protected members if there are no protected functions in the base class"

comment:10 in reply to:  7 Changed 14 years ago by Markus Ringnér

Replying to peter:

Ok, let's sober up and be specific rather than general.

I checked in yat and we have protected members in Kernel, SupervisedClassifier?, FeatureSelection?, Sampler, Discrete, Continuous, OneDimensional?, OneDimensionalWeighted?, Score, NNI.

In some cases I could agree with Markus points that it is convenient and preferable to have them as protected (e.g. Sampler) but in other cases, e.g. FeatureSelector?, members should not be touched by daughter classes but only read and it is stupid to have them as protected.

I think we can agree on that we need to go through these classes and change them when it sensible. I guess it is harder to agree on what is sensible. But if start with the obvious cases... the rest will follow(?)

Exactly, instead of saying protected members are ugly, we should think about interface design for derived classes every time we type protected.

comment:11 Changed 13 years ago by Jari Häkkinen

Milestone: 0.4later

comment:12 Changed 13 years ago by Peter

(in [1219]) refs #284 - made target_ in Sampler private

comment:13 Changed 9 years ago by Peter

Milestone: yat 0.x+
Resolution: wontfix
Status: newclosed

Nobody has looked at this for 5 yrs and as yat API has stabilized it would just create problems if we privatized protected members.

Note: See TracTickets for help on using tickets.