isocpp / CppCoreGuidelines

The C++ Core Guidelines are a set of tried-and-true guidelines, rules, and best practices about coding in C++
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
Other
42.48k stars 5.43k forks source link

Should there be guildlines for bounding friendship? #1303

Open wkaras opened 5 years ago

wkaras commented 5 years ago

Except for very simple classes, C++ friendship should be more like human friendship. The secrets we divulge to each non-stranger is distinct to that particular non-stranger. We generally don't want anyone to know all of our secrets. Could there be guidelines for capturing statically-checkable limits to friendship in code? Maybe following the pattern illustrated by this example: https://godbolt.org/z/Ztw-SQ

The caveat is that bounding friendship could be a crutch that enables overly complex classes. Programmers have a tendency to not break up large classes when they should. It's a partial backslide into the "it's easier if everything's global" philosophy.

cubbimew commented 5 years ago

Real-life uses of friendships I observe are based on functionality rather than metaphors and seem to cluster in a couple of idioms:

  1. (idiom) binary operators, because of conversion symmetry and because this hides them from non-ADL lookup, improving build time/diagnostics.
  2. (idiom) iterators and proxies (bitset's, vector<bool>'s): map does not offer user access to individual nodes of the rbtree it holds, and bitset doesn't expose its storage units, but objects of map::iterator and bitset::reference must access them
  3. (rare exception) functions that must have private access but can't be members. Examples std::any_cast (can't be a member because it takes a pointer to any) and std::call_once (can't be a member because once_flag can be non-class)

That said, "avoid long-distance friendship" is one of the old To-do items in these guidelines, currently under consideration to be dropped (https://github.com/isocpp/CppCoreGuidelines/issues/811#issuecomment-299991479)

wkaras commented 5 years ago

Example: a hotel class. The reserve operation on a hotel object returns a reservation object. You don't want check-ins to happen without reservations. The straight-forward implementation is that check-in is an operation on reservation, which is a friend of hotel. But it should be a limited friendship. A reservation object shouldn't be able to effect an order for new bed sheets. (The fly in the ointment is that reservation is naturally a member class of hotel, and thus would have (unlimited) friendship.)

In general, one should be cautious about conclusions based on one's experience (only). If I only considered my own experience, I could conclude that double would better be a library-provided type rather than part of the base language. Working on the STL seemed to convince Stepanov that virtual member functions were just cruft.

brenoguim commented 5 years ago

The straight-forward implementation is that check-in is an operation on reservation, which is a friend of hotel.

You don't check-in on a reservation. You check-in on a hotel, and possibly bring your reservation with you. So, another way to implement it would be to have a check-in at the hotel giving the reservation as a parameter.

Point being: instead of cluttering the code with "limiting friends" it could be that alternate designs exist where no friend is needed at all. And I strive to avoid friendship in real code, often coming to a better design. But there are some patterns I can't really get around the friendship without making the code significantly more complicated, which are the ones mentioned by @cubbimew .

wkaras commented 5 years ago

Yes you can make reservation some sort of simple handle, an put any operations with it in hotel instead. But that makes the interface to hotel larger and harder to understand and manage. (Perhaps an argument for namespaces within classes.) If one goes overboard in avoiding friendship, it makes the class public interface harder to understand and use.

Another variant on this pattern is when a class is more logically thought of as having multiple public interfaces (as opposed to an interface that is specifically for one other class). Consider a queue. It could have a friend with an enqueue operation and a (distinct) friend with a dequeue operation. The only data member for these friends would be a reference to the queue. Any advantage is small for such a simple class, but it could be an important advantage for more complex classes.

neithernut commented 5 years ago

Another variant on this pattern is when a class is more logically thought of as having multiple public interfaces

You'd probably use some variant of "traits" in such a situation. If you really need it (some situation like this may as well be an indicator of a broken architecture).

hsutter commented 5 years ago

Editors call: We agree there could be a short guideline here based on Sergey's reply above, even though the guideline won't be enforceable. See also these links:

https://blogs.msdn.microsoft.com/vcblog/2018/11/20/qa-fine-grained-friendship/ http://www.drdobbs.com/friendship-and-the-attorney-client-idiom/184402053

wkaras commented 5 years ago

Interesting, the Dr. Dobbs article seems to be describing a sort of bounded friendship. The author prefers static member functions in the mediating class, that take a reference to the grantor, rather than having a reference to the grantor as a data member. But I can't think of a concise set of guidelines to capture this advice.