How to correctly set up interaction from a variable declared in a class?

Question

C++ / C#

At the moment, the abstract class interface is written:

class InstrumentInterface {
public:
  virtual ~InstrumentInterface() = default;
  virtual bool getLock() = 0;
  virtual std::string writeCommand(std::string*) = 0;
  virtual std::string readCommand() = 0;
protected:
  std::mutex interfaceControl;
};

From it I define methods:

class InstrumentalServer : public InstrumentInterface
{
public:
  InstrumentalServer() = default;
  virtual ~InstrumentalServer();

  void initSocketServer();

  void addDevice(AbstractDevice*);

  virtual bool getLock() override;
  virtual std::string writeCommand(std::string*) override;
  virtual std::string readCommand() override;
private:
  SocketServer* sockServer;
  AbstractDevice* device;
};

I pass this inside the SocketServer class to be able to interact with AbstractDevice (there will be 2 more servers and it is pointless to pass AbstractDevice to the server, since the InstrumentalServer class must control access to AbstractDevice ):

void InstrumentalServer::initSocketServer()
{
  sockServer = new SocketServer(this);
}

Inside the SocketServer class, in a separate server thread, I call interface functions:

try
{
  Interface->writeCommand(buf);
}
catch (const std::exception& ex)
{
  std::cerr << ex.what() << std::endl;
}

How correct is this? What is the correct way to send data to InstrumentalServer from SocketServer?
Thanks for answers!

Answer 1

I will answer as I understand the question, maybe even by.
The main problem is strong connectivity. You have a lower-level SocketServer class for some reason must know about the existence of the InstrumentInterface that owns it.
I would suggest passing "something" to the server in the constructor, where he can write the received data. A simple example - InstrumentalServer waits until data arrives from the server and writes it to the device. A simple example that runs forever - the server wakes up once a second, writes data and goes to sleep. When data appears, the InstrumentalServer wakes up, writes them to the device, and falls asleep, again waiting for incoming data. The classic multi-threaded problem is the same. Removed methods from the interface.

struct MessageQueue
{
    std::condition_variable cv;
    std::mutex messMtx;
    std::queue<std::string> serverMessages;
};

struct AbstractDevice
{
    void processData(std::string message)
    {
        std::cout << message << std::endl;
    }
};

class SocketServer
{
public:
    SocketServer(MessageQueue& mess)
        : out(mess)
    {
        std::thread t(&SocketServer::processData, this);
        t.detach();
    }

    void processData()
    {
        for (;;)
        {
            try
            {
                std::lock_guard<std::mutex> lock(out.messMtx);
                // some output data
                out.serverMessages.push("some string 1");
                out.serverMessages.push("some string 2");
                // completed this iteration
                out.cv.notify_one();
            }
            catch (const std::exception& ex)
            {
                std::cerr << ex.what() << std::endl;
            }
            std::this_thread::sleep_for(std::chrono::seconds(1));
        }
    }

private:
    MessageQueue& out;
};

class InstrumentInterface {
public:
    virtual ~InstrumentInterface() = default;
};

class InstrumentalServer : public InstrumentInterface
{
public:
    InstrumentalServer() = default;

    void addDevice(AbstractDevice* dev) { device = dev; };

    void waitForMessages()
    {
        for (;;)
        {
            std::unique_lock<std::mutex> lock(messages.messMtx);
            // sleep until messages received
            messages.cv.wait(lock, [this]() {return !messages.serverMessages.empty(); });
            while (!messages.serverMessages.empty())
            {
                device->processData(messages.serverMessages.front());
                messages.serverMessages.pop();
            }
        }
    }

private:
    MessageQueue messages;
    std::unique_ptr<SocketServer> sockServer = std::make_unique<SocketServer>(messages);
    AbstractDevice* device;
};

int main()
{
    AbstractDevice device;
    InstrumentalServer iServer;
    iServer.addDevice(&device);
    iServer.waitForMessages();
    return 0;
}

It would be possible to make a separate mutex for the device, of course, so as not to capture the message mutex for a long time. Then we create an empty queue under the message mutex, swap it with the message queue, release the message mutex, grab the device mutex, write to the device.
Did I understand the question correctly?