views:

468

answers:

9

Although class friendship is one of the last resorts of C++, does this pattern make sense?

class Peer
{
public:
    friend class Peer;
    void GetSecret(const Peer& other)
    {
     const std::string& secret = other.GiveSecret();
     std::cout << secret << std::endl;
    }

private:
    const std::string& GiveSecret() const
    {
     return "secrety stuff";
    }
};

int main(int argc, char* argv[])
{
    Peer peerA;
    Peer peerB;
    peerA.GetSecret(peerB);
    return 0;
}

Ok the reason for this pattern is because the Peers are all of the same rank, and they need to share knowledge among each other, however this knowledge is secret, because no one but the peers should use it, or the program is no longer valid.

One very real example of this is that when one peer is copy-constructed from another peer it needs to access secret information from its source peer, but again there is no reason for anyone else to know about these internals, just the peers.

A: 

Not healthy, it breaks the the encapsulation best practice

Ido
A: 

does this pattern make sense

Not much. Can you elaborate?

  • You create friendship typically among different classes. This is syntactically valid. Why would Peer be a friend of Peer?
  • Private functions can only be used by member functions. Calling them from client code is wrong.

As to the question if friendship breaks encapsulation or not, have a look at the FAQ.

Sorry Robert, misread the names.

dirkgently
It does compile. GetSecret is public. GiveSecret is private. And of course I compiled this before asking
Robert Gould
+17  A: 

friend is not necessary in this case. An object of a class can access the private members of any other object of the same type. It should work just fine without the friend declaration.

KeithB
incredible! you are right, the friendship declaration isn't necessary. I could've sworn I couldn't access another's private methods even with an object of the same class. Learn something everyday, even after many years of C++!
Robert Gould
A class is always a friend of itself.
Martin York
+1  A: 

I always wondered if this feature was part of the specifications or just implementation-specific. I always felt too lazy to check though.

Any hints on that ?

Benoit Myard
A: 

Not healthy, it breaks the the encapsulation best practice

Well, yes and no.

It can only break encapsulation if you do not use methods to access the data you need.

Not to say that encapsulation is only a way to protect foreign classes to have to be rewritten in case you change implementation details. If you happen to change the implementation, you'll have to rework your class anyway.

Benoit Myard
+3  A: 

standard c++ says that the private clause has class scope. That's means that every Peer can access the private part of any other Peer. This isn't implementation specific

thrantir
+2  A: 

Although you have discovered that friendship is not needed in this case, I'll just say that contrary to the above opions there is nothing wrong in principle with friendship between cooperating class. In fact rather than breaking encapsulation the friendship might actually promotes it.

Consider the alternative where you create an accessor method to your private data. If you do that then you're effectively giving access to the private data of all clients not just the limited set of classes/functions declared as friends. If only one method in your friend class is going to access your internals encapsulation has decreased by exactly the same amount as if you had provided a public accessor method. However, the result of giving a public accessor method will be that far more clients will use method.

For each application class we write there is a shadow class which is the classes unit test. The unit test class is a friend of the application class as it frequently needs to invoke the methods on the class and then examine the class internals, or invoke private methods. By being a friend class encapsulation is maintained.

Anyway for a good discussion see here: http://www.ddj.com/cpp/184401197

lilburne
A: 

First I want to say that with inheritance, a good class hierarchy does not need to expose private data in order for things such as copy constructors or operator overloads to work. For example:

class Sub
    : public Base
{
public:
    Sub(const std::string & name)
        : Base(),
          m_name(name)
    {
    }

    Sub(const Sub & src)
        : Base(src),
          m_id(src.m_name)
    {
    }

    Sub & operator=(const Sub & rhs)
    {
        if (&rhs != this)
        {
            Base::operator=(rhs);

            m_name = rhs.m_name;
        }

        return (this);
    }

protected:
    virtual Debug(void)
    {
     std::cout << "Sub [m_name = " << m_name << "]" << std::endl
               << "+- ";

     Base::Debug();
    }

private:
    std::string m_name;
};

class Base
{
public:
    Base(int id)
        : m_id(id)
    {
    }

    Base(const Base & src)
        : m_id(src.m_id)
    {
    }

    Base & operator=(const Base & rhs)
    {
        if (&rhs != this)
        {
            m_id = rhs.m_id;
        }

        return (this);
    }

protected:
    virtual Debug(void)
    {
     std::cout << "Base [m_id = " << m_id << "]" << std::endl;
    }

private:
    int m_id;
};

In this example, the subclass is able to instantiate its base members in both the copy constructor and the assignment operator. Even the overly contrived Debug method works without ever having access to the private members of the base class.

Mike
+1  A: 

Now that I've said my piece on inheritance, here's something that might help with your real question, i.e. how to get around the potential problems with friendship.

The way I do this is to create a pure interface for accessing data I want to share with my "friends". I then implement this interface privately, so nobody can access it directly. Finally I have some mechanism that allows me to pass a reference to the interface only to those select classes that I want to allow.

For example:

// This class defines an interface that allows selected classes to
// manipulate otherwise private data.
class SharedData
{
public:
    // Set some shared data.
    virtual void SettorA(int value) = 0;

    // Get some shared data.
    virtual bool GettorB(void) const;
};


// This class does something with the otherwise private data.
class Worker
{
public:
    void DoSomething(SharedData & data)
    {
     if (data.GettorB() == true)
     {
      data.SettorA(m_id);
     }
    }

private:
    int m_id;
};

// This class provides access to its otherwise private data to
// specifically selected classes.  In this example the classes
// are selected through a call to the Dispatch method, but there
// are other ways this selection can be made without using the
// friend keyword.
class Dispatcher
    : private SharedData
{
public:
    // Get the worker to do something with the otherwise private data.
    void Dispatch(Worker & worker)
    {
     worker.DoSomething(*this);
    }

private:
    // Set some shared data.
    virtual void SettorA(int value)
    {
     m_A = value;
    }

    // Get some shared data.
    virtual bool GettorB(void) const
    {
     return (m_B);
    }

    int    m_A;
    bool   m_B;
};

In this example, the SharedData is an interface that determines what can be done with the data, i.e. what can be set, and what is get-only. The Worker is a class that is allowed access to this special interface. The Dispatcher implements the interface privately, so having access to a Dispatcher instance doesn't give you access to the special shared data, but the Dispatcher has a method that lets Workers get access.

Mike