Correctly perform dynamic cast

Question

In Setting a variable in a child class, I was trying to figure out how to correctly derive variables in polymorphic classes. After some help, I found out that I needed to use a dynamic_cast on a pointer to correctly access the information I need. I am having some trouble with this.

This is the function I am currently working on.

void translateLines(Parser parser, Code code)
{
    while(parser.hasMoreCommands())
    {
        vector<Command>::const_iterator it = parser.currentCommand();
        if(it->commandType() == "A")
        {
            //SubType* item = dynamic_cast<SubType*>(*the_iterator);
            A_COMMAND* a_command = dynamic_cast<A_COMMAND*>(*it); //line that is throwing the error
            //string symbol = a_command->get_symbol();
            //cout << "symbol: " << symbol << endl;
            //perform binary conversion
        }
        /*else if(command.commandType() == "C")
        {
            string dest = command.get_dest();
        }*/
         //shouldn't be any L commands in symbol-less version
        else
        {
            std::cout << "unexpected command value \n";
        }
        parser.advance();
    }

}

This is my Parser.h, which has the relevant information regarding the iterator for the vector.

#include "Command.h"
#include <vector>


class Parser {
private:
    std::vector<Command> commands;
    std::vector<Command>::const_iterator command_it = commands.begin();
public:
    Parser(std::vector<std::string>);
    bool hasMoreCommands() //are there more commands in the input?
    {
        if(command_it != commands.end())
            return true;
        else
            return false;
    }
    void advance(){std::next(command_it);} //move to next command, should only work if hasMoreCommands returns false}
    std::vector<Command>::const_iterator currentCommand(){return command_it;}
    std::vector<std::string> translateCommands(); //convert commands into binary strings

};

Here is the error I am receiving:

g++ -O0 -g3 -Wall -c -fmessage-length=0 -std=c++11 -o Assembler.o "..\\Assembler.cpp" 
..\Assembler.cpp: In function 'void translateLines(Parser, Code)':
..\Assembler.cpp:32:55: error: cannot dynamic_cast 'it.__gnu_cxx::__normal_iterator<_Iterator, _Container>::operator*<Command*, std::vector<Command> >()' (of type 'class Command') to type 'class A_COMMAND*' (source is not a pointer)
    A_COMMAND* a_command = dynamic_cast<A_COMMAND*>(*it);
                                                       ^

Any clue what's wrong here?

EDIT: So I see now that I can't use a vector of Commands, rather I need pointers to the commands. I already have changed Parser.h to handle vector<Command*> rather than vector<Command>. For the input I tried something like this:

A_COMMAND command();
commands.push_back(&command);

But this isn't quite working for me, as the vector is expecting pointers and not references. What would be the easiest way to create a pointer to the memory and push it into the vector?

Answer 1

You have a vector of Commands. You cannot cast a Command to an A_COMMAND*. It's important to note that a vector<Command> cannot possibly contain a A_COMMAND. If you want to do runtime polymorphism in C++, you have to use pointers or references. In this case your Parser::commands would need to be a std::vector<Command*> (or some type of smart pointer like std::vector<std::shared_ptr<Command>>).

Take for example this code:

std::vector<Command> commands;
A_COMMAND a_command;
commands.push_back(a_command);

commands does not contain an A_COMMAND object. It contains a Command object that is a copy of a_command. It more-or-less equivilant of this:

std::vector<Command> commands;
A_COMMAND a_command;
Command temp(a_command);
commands.push_back(temp);

Remember, in C++ a variable is an object, not a reference to an object like in some other languages (Java or C# for instance). Objects will never change type, but you can have a reference or pointer of one type that points to an object of a derived type:

std::vector<Command*> commands;
A_COMMAND a_command;
commands.push_back(&a_command);

In this case commands[0] is a Command*, but it points to an A_COMMAND object.

RE your edit:
You are adding a pointer. &some_variable returns a pointer to some_variable, BUT you should absolutely never, ever do something like that. As soon as command goes out of scope, it will be destroyed and any access to it will result in undefined behavior. You will need to use dynamic memory allocation with new. It would probably be best to use a smart pointer class like std::shared_ptr<Command> to hold your dynamically allocated objects so you don't have to worry about deleteing them later.

If you use raw pointers then something like this will work:

A_COMMAND* command = new A_COMMAND;
commands.push_back(command);

If you go with that approach, you'll need to delete all of your commands when you're done with them (probably Parser's destructor):

for(Command* command : commands) {
    delete command;
}

It would be better to use std::shared_ptrs though. Declare commands as std::vector<std::shared_ptr<Command>> commands;, then:

std::shared_ptr<A_COMMAND> command = std::make_shared<A_COMMAND>();
commands.push_back(command);

Then your objects will all get automatically deleteed when the last shared_ptr to them goes out of scope. If you use smart pointers you'll need to cast them slightly differently though. Look into std::dynamic_pointer_cast.

Answer 2

The real question is why use dynamic_cast at all. This is a job for virtual methods. If another class is derived, then your dynamic_cast will also need updating, with a virtual method you do not need to be concerned what the derived class is, only that it overrides the virtual method, which can be forced by using an interface class for the base (pure virtual methods, no state). This sounds like an application for the strategy pattern. https://en.wikipedia.org/wiki/Strategy_pattern.

Answer 3

try (it) instead of (*it) the iterator should be a pointer to the object allready so you need to omit the * as this would result in the actual data not the reference