CODEWITHSUNDEEP
c++oopdowncast
Flyboy Wilson
Flyboy Wilson

Reputation: 49

Avoid both static & dynamic casting

I'm updating a system that receives multicast messages, parses the data into classes, then passes a base class pointer to a separate thread through a queue. The other thread then reads the data from the classes, and stores them in tables.

There are 2 different kinds of messages we receive. Here's the class design:

class BaseMsg
{
public:
    BaseMsg(char tp) : msgType(tp) {}
    virtual ~BaseMsg() = 0;

    char msgType;
}

class MsgType1 : public BaseMsg
{
public:
    MsgType1( int a, int b) : BaseMsg('1'), val1(a), val2(b) {}
    virtual ~MsgType1();

    int val1, val2;
}

class MsgType2 : public BaseMsg
{
public:
    MsgType2( double a, double b) : BaseMsg('2'), val1(a), val2(b) {}
    virtual ~MsgType2();

    double val1, val2;
}

Here's the receive code:

void reciveMessage( char *datablob, MsgQueue *queue)
{
    BaseMsg *msgReceived;
    char type = datablob[0];    // message type is the first character in the data,
                                // the rest of the blob varies based on that type
    if ( type == '1' ) {
        // code for parsing type 1 into 2 ints, i1, i2
        msgReceived = new MsgType1( i1, i2);
    } else {
        // code for parsing type 2 into 2 doubles, d1, d2
        msgReceived = new MsgType2( d1, d2);
    }
    queue->push(msgReceived);
}

Here's the process code which runs in a separate thread:

void processMessage(MsgQueue *queue)
{
    BaseMsg *msgToProcess = queue->pop();

    if ( msgToProcess->msgType == '1' ) {
        MsgType1 *mt1 = static_cast<MsgType1 *>(msgToProcess);
        // do stuff with the message type 1 data
    } else {
        MsgType2 *mt2 = static_cast<MsgType2 *>(msgToProcess);
        // do stuff with the message type 2 data
    }
}

I know that the check for message type then downcasting is sloppy, but given the contraints of communicating through the queue, I could not come up with a better solution. Even using dynamic_cast<> (which I don't want to do for performance reasons) would entail the same issue; it still needs to know what type of class to convert msgToProcess into.

Any suggestions about how I could get rid of the checks & casting? I have a lot of experience with C & C++, but not much with OO design, so there may be a way I know nothing about.

Also note that this is an extremely stripped-down example to illustrate the question. There are actually over 50 different message types, and performance is critical, as we can receive millions of messages per second.

Upvotes: 2

Views: 149

Answers (2)

n.caillou
n.caillou

Reputation: 1342

I agree with others who have suggested to use the inherent properties of polymorphism and use member functions.

But I also understand the need to keep the message classes pretty clean. I think using a Visitor Pattern (that relies on polymorphism) may be a good idea in this case:

class BaseMsg {
public:
    virtual void accept(MsgProcessor& p) = 0;
};

class Msg1 : BaseMsg {
public:
    void accept(MsgProcessor& p) { p.process(*this); }
};

class Msg2 : BaseMsg {
public:
    void accept(MsgProcessor& p) { p.process(*this); }
};

class MsgProcessor {
public:
    void process(Msg1& m);
    void process(Msg2& m);
}

The cool thing is that this will not compile if one of the functions is missing for any of the derived classes.

edit: Fixed stuff pointed out in comments

edit2: with two codebases & minimal changes in the external codebase:

External codebase:

class MsgVisitor;
class BaseMsg {
public:
    virtual void accept(MsgVisitor&) = 0;
};

class Msg1;
class Msg2;
class MsgVisitor {
public:
    virtual void visit(Msg1&) = 0;
    virtual void visit(Msg2&) = 0;
};

class Msg1 : public BaseMsg {
public:
    void accept(MsgVisitor& v) { v.visit(*this); }
};

class Msg2 : public BaseMsg {
public:
    void accept(MsgVisitor& v) { v.visit(*this); }
};

Local codebase:

class MsgProcessor : public MsgVisitor {
public:
    void visit(Msg1& m) { ... } // do stuff for Msg1
    void visit(Msg2& m) { ... }
};

void processMessage(MsgQueue *queue)
{
    BaseMsg *msgToProcess = queue->pop();
    msgToProcess->accept(MsgProcessor());
    //delete msgToProcess;
}

Upvotes: 3

Robert Kock
Robert Kock

Reputation: 6038

I would define a pure-virtual function called for example, doStuff() within the class BaseMsg.
Each derived class implements that method.
Within your processMessage, you simply call msgToProcess->doStuff()

Upvotes: 2

Related Questions