Client-server application design issue

Question

I have a collection of clients on server's side. And there are some objects that need to work with that collection - adding and removing clients, sending message to them, updating connection settings and so on. They should perform these actions simultaneously, so mutex or another synchronization primitive is required. I want to share one instance of collection between these objects, but all of them require access to private fields of collection. I hope that code sample makes it more clear[C++]:

class Collection
{
    std::vector< Client* > clients;
    Mutex mLock;
    ...
}

class ClientNotifier
{
    void sendMessage()
    {
         mLock.lock();
         // loop over clients and send message to each of them
    }
}

class ConnectionSettingsUpdater
{
    void changeSettings( const std::string& name )
    {
        mLock.lock();
        // if client with this name is inside collection, change its settings
    }
}

As you can see, all these classes require direct access to Collection's private fields.

Can you give me an advice about how to implement such behaviour correctly, i.e. keeping Collection's interface simple without it knowing about its users?

Answer 1

From your limited code snippet, what I understand is that the Collection class is a data-centric class that holds a vector of Client* and so on. The other 2 classes are method-centric and use a Collection object (not sure how, but presumably the object is a member of this class or is being passed to the methods somehow).

My suggestion would be to merge these data-centric and method-centric classes. The Collection class correctly represents a collection of Clients so that design is spot-on. But the other classes are actually performing operations on the Collection - which is why you end up needing access to private members. Any operations that are performed on an instance of a class should be member functions of that class itself.

Thus, sendMessage() should be a member of the Collection class, since you are sending a message to all Clients in the Collection.

Now, that might seems like a naive suggestion since it looks like you are trying to group together methods related to notifying the clients in the ClientNotifier class. If you want to stay with this approach then here's a solution that might help:

class Collection
{
    private:
        std::vector< Client* > m_clients;
        Mutex m_mLock;

    public:
        void sendMessage()
        {
            ClientNotifier objClientNotifier;  // Create an instance of the ClientNotifier class that provides this functionality

            /* Loop through all clients in the member vector to send a message to each of them */
            for (std::vector<Client*>::iterator itrClients = m_clients.begin(); itrClients != m_clients.end(); ++itrClients) {
                m_mLock.lock();  // Member mutex access through a member function, which is acceptable
                objClientNotifier.sendMessage(*itrClients);
                m_mLock.unlock();
            }
        }
}

class ClientNotifier
{
    public:
        void sendMessage(Client* pobjClient)
        {
             /* Use the pointer to a Client object to send a message to that client */
        }
}

Answer 2

You could implement a simple locking function and iteration functions to avoid exposing the private variables. If you want to reuse it, you could use a mixin pattern. For example (pseudo code):

class Collection {
    bool getLock(){};
    bool unlock(){};
    Client* getClient(){};
    Client* getNextClient(){
        // returns null when there are no more clients
     }
 }

A search for mixins will show you how to abstract this idea.