4

I want to make a multi-threaded model in which the server main loop will not be stalled by pending database transactions. So I made a few simple classes, this is a very simplified version:

enum Type
{
    QueryType_FindUser      = 1 << 0,
    QueryType_RegisterUser  = 1 << 1,
    QueryType_UpdateUser    = 1 << 2,
    //lots lots and lots of more types
};

class Base
{       
public:
    Type type;
};
class User: public Base
{   
public:
    std::string username;
    User(std::string username)
        :username(username)
    {type = QueryType_FindUser;}        
};

Now when I transfer the data as a Base, I want to convert it back to User class again:

concurrency::concurrent_queue<QueryInformation::Base> BackgroundQueryQueue;
void BackgroundQueryWorker::Worker()
{
    while(ServerRunning)
    {
        QueryInformation::Base Temp;
        if(BackgroundQueryQueue.try_pop(Temp))
        {
            switch(Temp.type)
            {
                case QueryInformation::Type::QueryType_FindUser:
                {
                    QueryInformation::User ToFind(static_cast<QueryInformation::User>(Temp));//error here
                    //do sql query with user information
                    break;
                }
                //etc
            }
        }
        boost::this_thread::sleep(boost::posix_time::milliseconds(SleepTime));
    }
}

Where I marked the //errorhere line it says that there is no user-defined conversion from Base to User, what should I do? How can I define this conversion?

I'm new to polymorphism so an additional explaination why this doesn't compile would be also great :) From what I understood about polymorphism it should be doable to convert freely between base<->derived..

Gizmo
  • 1,990
  • 1
  • 24
  • 50
  • hm, I cant remember when I type-casted the last time without using pointers ( eg `static_cast(&Temp)` ) . Sadly I could not find the corresponding manual pages and its friday ( brain empty ) – Najzero Jul 05 '13 at 09:31

2 Answers2

4

You can only use polymorphism by using pointers or references, not direct values.

hence, you should use:

QueryInformation::User * user = static_cast<QueryInformation::User *>(&Temp);

and modify your code accordingly to use the pointer instead of the direct value.

Furthermore, the way you allocated Temp, it is indeed of class Base, not of class User, hence its "type" will not be QueryType_FindUser, and then static_cast will not be perform (you will not enter the case value).

The normal use of polymorphism with static_cast should look like this:

QueryInformation::Base * temp = new QueryInformation::User; 
// or obtain by some other methods, so that you don't know the exact
// type, but anyway you *must* use a pointer (or reference).

switch(temp->type)
{
case QueryInformation::Type::QueryType_FindUser:
    {
        QueryInformation::User * user = static_cast<QueryInformation::User*>(temp);
    }
}

In your case, this means instead of having:

concurrency::concurrent_queue<QueryInformation::Base> BackgroundQueryQueue;

you should have (note the pointer):

concurrency::concurrent_queue<QueryInformation::Base *> BackgroundQueryQueue;

And as Mats suggested, it is probably even better to call a virtual method of Base that will do the dispatch automatically, instead of creating yourself a switch and making manually a static_cast.

Boris Dalstein
  • 7,015
  • 4
  • 30
  • 59
  • 1
    Technically, this won't work, since the stored object has already been sliced and the "extra" bits in `User` have gone missing. – Mats Petersson Jul 05 '13 at 09:34
  • yes, I believe it would be best to mark Base class abstract ( enforcing pointers and everything else pretty much to be in the correct design ) – Najzero Jul 05 '13 at 09:39
  • Nitpicking: polymorphism also works with references, not only pointers. – syam Jul 05 '13 at 09:41
  • @MatsPetersson, I edited my answer to reflect this. The code was *semantically* wrong, but technically correct. Just the static_cast would never occur in the first place since the Base was *not* of type User. – Boris Dalstein Jul 05 '13 at 09:41
  • @syam: yes, I assumed this was obvious since references are pointers underneath (and I actually used references in my code..) but better make it clear. – Boris Dalstein Jul 05 '13 at 09:44
  • when I `delete user` when done (in you second example) will the memory be totally freed by that one object? What should I use for auto memory management? auto_ptr? weak_ptr? lazy_ptr? shared_ptr? Unique_ptr? There are too many :/ – Gizmo Jul 05 '13 at 10:16
  • @Gizmo: I am not an expert in smart pointers, but to the best of my knowledge, I would recommend you use `unique_ptr`: `concurrent_queue< std::unique_ptr >`. This means the ownership of your pointers is only retained by the queue, and the memory will be released as soon as the queue is destroyed. If you pop the element from the list, then you should move the ownership to the caller of "pop", like [this](http://stackoverflow.com/questions/14127487/remove-unique-ptr-from-queue), then perform the static cast like this: `User * user = static_cast(q2.get());`. – Boris Dalstein Jul 05 '13 at 10:52
  • @Gizmo Oh, and in any ways, you *must* use a [virtual destructor](http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors) for Base. Hence, when `delete base` is called (either by yourself or by the smart pointer), the appropriate destructor will be called. (for instance, ~User) – Boris Dalstein Jul 05 '13 at 11:05
  • unresolved symbol public: virtual __thiscall ResultInformation::Base::~Base(void) when I declare `virtual ~Base();` :( – Gizmo Jul 05 '13 at 22:03
  • ah nvm `virtual ~Base(){}` is Ok. – Gizmo Jul 05 '13 at 22:06
  • @Gizmo yep, if you declare a non-default desctructor, you *must* also provide the implementation. [*Even in the case when you declare it pure virtual*](http://stackoverflow.com/questions/630950/pure-virtual-destructor-in-c) (this can be surprising at first). – Boris Dalstein Jul 05 '13 at 22:21
1

To me, this seems all a bit wrong.

First, your data structure concurrent_queue<QueryInformation::Base> is storing the Base object, which means that any additional information in the actual object (e.g. User) will be lost due to slicing. You need to use a (smart) pointer to store the address of a Base object - which will then be able to hold any size object.

Second, surely, this should use virtual functions to achieve the required tasks. If you store what type object it is and then call dynamic_cast to convert the object, it's "wrong". Sure, there are some rare circumstance when this is the right thing to do, but most of the time, dynamic_cast (or static_cast) to convert between one object and another is a "bad smell", and as you probably know, bad smells should be avoided.

Mats Petersson
  • 126,704
  • 14
  • 140
  • 227
  • +1 I added this recommendation to my own answer since this is the one the OP has accepted (didn't use a smart pointer for clarity). – Boris Dalstein Jul 05 '13 at 10:00