[C++] friend keyword - bad programming style?

Started by
14 comments, last by dmatter 16 years, 7 months ago
Hello, this is actually my first post on the forums. Regarding the topic: is using the friend keyword considered bad OOP style? I heard that some people refer to it as hack, others say that isn't bad at all. The reason I ask is because I tend to use this keyword frequently. Currently I am working on a physics engine and I have this class that holds an array of all the rigid bodies, contacts, joints etc in the simulation. The bodies, contacts etc are classes themselves and have a lot of members that I don't want to make publicly accesible. But with regards to the actual simulation, I need access to these private members from within the main world-class, thus I used the friend keyword. But I am not too sure whether this is good coding practice or not. Another option would have been either writing a lot of get/set routines, or to shift things around, like giving the Body-class the method for detecting collisions, but this would require, that the body class had access to the contacts class' private members... So the decision to use the friend keyword seemed pretty sensible to me. What do you guys think? Is it ok to use the friend keyword in this case and also in general?
Advertisement
Yes. Why not just make fields public?

It doesn't come down to the friend keyword which is perfectly valid. It comes down to OO design.

Many languages do not support hacks like that. Once something is private, it's private.

Get/set routines don't solve anything either, they are just a different syntax for friend.

You might want to reconsider the design. While adding friend for certain specific small parts is just fine, abusing it is bad. Reason for this is that it doesn't lend itself to extensibility. And since your design isn't extensible, might as well put everything at same level.

If you have lots of classes with lots of friend accesses, you might want to put these classes to internal use only, and make everything about them public.

Quote:is using the friend keyword considered bad OOP style


Technically, it's not even OO. Strict OO doesn't allow access to non-visible members at all. In reality however, it's practical to have that ability.
friendship is directed destruction of encapsulation (it provides a third party access to implementation detail), and as such should be used with caution. In complete isolation, on its own merits, friendship isn't neccessarily evil or wrong -- what makes it evil or wrong is the context.

However, if you're using it often, that probably means you're using it poorly.

Quote:
I need access to these private members from within the main world-class, thus I used the friend keyword.
But I am not too sure whether this is good coding practice or not.

This is good example of where friendship is a bad idea. You're exposing implementation detail to objects that really don't require it; all you're gaining is a bit of short-term convienience. Provide a more robust public interface for the rigid bodies instead.
simple answer: NO!

same goes for GOTO:) to all those who hate goto's: basically even the most advanced OOP code is a collection of goto's :D

now - if u find yourself really abusing of such features, i mean if u know it's a better way to do things then it's bad practice.
by 'better' i mean something not born out of laziness and "i'll change it later, this is just for testing" type-of thinking
'friend' isn't bad itself, but it's often a symptom of bad design elsewhere. If you've got a load of private data and some code needs to operate on that data, that code should be a member of the same class (usually).

Richard "Superpig" Fine - saving pigs from untimely fates - Microsoft DirectX MVP 2006/2007/2008/2009
"Shaders are not meant to do everything. Of course you can try to use it for everything, but it's like playing football using cabbage." - MickeyMouse

Friends are good! (Well..yes and no)

Why?

Well there are 3 reasons to use the friend keyword:

  1. Sometimes it's the only way to do something

  2. Sometimes it's the cleanest way to do something

  3. Most times it's simplest way to connect classes within an existing poorly designed code base.
Point 1 is quite rare you would be hard pressed to think up times when there is little to no alternative. You might (or might not) consider operater overloading to be a case for this.

Point 2 is occasional, this is the inteded use of the friend keyword, rather than break encapsulation you actually increase it; the alternative would have been public get/set methods.

Point 3 is the most common use of the friend keyword, if you need two classes to communicate within already poorly designed code then this is the easiest way to do it; there's undoubtedly a better way to achieve the same thing but it requires redesigning your code, not an option if you're on a tight deadline. You may be able to avoid using friend in this case but only at the expense of writing yet more bad code.

So why does everyone say they're bad? Read on.

Many people who use friends excessively have a smelly design (as in point 3) and don't realise this; and so they assume that in their case they are correctly using friends as per point 2, of course they one day come back to their code and think 'oh-my, what an awful design' [wink]

As a rule of thumb if you're using the friend keyword and you don't know if it's the best thing or not then probabalistically you fit into point 3, this is why we generally urge to avoid them and instead consider a better design.

In your case it's likely that you're missusing the keyword, a better public interface is likely the best option.

In general if you have a bad design there are things you can consider which may help improve it:
  • If accessing a private function then consider whether you should make it public.
  • Consider whether the data that one friend class is accessing from the other would be better off in the accessing class in the first place.

  • Consider whether the function that one friend class is performing using the data from the other class would be better off as a public method of the other class.

  • Consider whether you definately require two classes, perhaps you're needlessly splitting a single responsibility between two classes.

  • Look at your problem from a higher or more abstract perspective, ensure it is well defined. Perhaps there's an existing design-pattern you can employ.

  • Research how others have solved your problem.


[Edited by - dmatter on September 17, 2007 11:26:05 AM]
IMHO, the whole idea of 'friends' just breaks the purpose of OO. Might as well go back to C instead of C++.

Whenever I am tempted to use them, I sit back and look at the design, and so far there has always been a design solution. I don't use them.

--random
--random_thinkerAs Albert Einstein said: 'Imagination is more important than knowledge'. Of course, he also said: 'If I had only known, I would have been a locksmith'.
I like the way c++ faq lite talks about "friend"
Quote:Original post by janta
I like the way c++ faq lite talks about "friend"


Thanks for the excellent link, also illustrating that the correct use of 'friends' within an OO paradigm is something best left to experts. I know that the C++ Standard Library uses them, and Stroustrup gives copious examples, but IMHO their use is best left to more advanced users, who understand the 'nuances' in an OO environment. In this thread, just about all the posters are in favour of their (infrequent) use, but with a lot of caveats.

I do concede one point of the article, though, that is that if the only alternative is to have public data in a class, the better alternative would be to use friends. In this way they do enhance encapsulation.

--random
--random_thinkerAs Albert Einstein said: 'Imagination is more important than knowledge'. Of course, he also said: 'If I had only known, I would have been a locksmith'.
Quote:Original post by jpetrie
This is good example of where friendship is a bad idea. You're exposing implementation detail to objects that really don't require it; all you're gaining is a bit of short-term convienience. Provide a more robust public interface for the rigid bodies instead.


Well, the problem is, that the world-class is responsible for collision detection and response, it detects collisions using the vertex data of the rigid bodies (this is why I need the friend keyword). I can't make the vertex data public, because it has to be constructed in a special way using AddVertex methods (to determine concave polygons and split them up into convex ones), so making them public could mess with the simulation.

Everything else has already public accessing methods.

I guess I then have to move the collision code inside the body class, although I didn't want the collision detection code to be public =/

Thanks for all those replies, I will now rethink my design to eliminate those "friends".

I didn't have to use the friend keyword before, as I am usually working alot with interfaces and the implementations have the actual data, so I can make more things public without thinking that improper use could break something (I don't mean making the data public, but several methods that otherwise would be private).
If the body-class were to be a interface I would give the implementation class the collision detection code and in actual use it would seem like the world class was handling the collision detection. But because of performance reasons I decided not to use inheritance with virtual classes. Accomplishing the same without interfaces seemed like making the collision detection private and defining the world class as a friend to the body class, because I didn't want the collision detection to be accesible from the outside as the world class should be responsible for handling collisions.

Edit:
Quote:I do concede one point of the article, though, that is that if the only alternative is to have public data in a class, the better alternative would be to use friends. In this way they do enhance encapsulation.

Hey, that was my reasoning why I ended up using the friend keyword. Now I don't know if I should keep the keyword or not =/

This topic is closed to new replies.

Advertisement