Re: Trying to call a function pointer causes a compile error with
GNU G++
Alex Buell wrote:
On Fri, 26 Dec 2008 01:20:19 +0100, I waved a wand and this message
magically appears in front of Thomas J. Gritzan:
std::map<ELEMENT, int>* commands;
If you have a pointer in your class and allocate memory for it, you
need to free it in the destructor, and you need to define a copy
constructor and a copy assignment operator.
That's odd, I'm definitely sure I did put code to delete the pointer in
the destructor. I use valgrind to find these sort of memory leaks.
You did delete it, but the copy and assignment weren't there.
Forbidding copy and assignment would work, of course.
Command::Command(std::vector<std::string>& arguments)
{
typedef std::pair<ELEMENT, int> command;
commands = new std::map<ELEMENT, int>;
cmd = commands->end(); // make sure it's set to the end
for (int i = 0; i < COMMANDS; i++)
commands->insert(command(command_table[i], i));
You can use std::make_pair here.
What's the difference between that and what I've used?
As Rolf Magnus wrote, you don't need the typedef. I wondered what you
need the typedef for as I looked over the code.
assert((int)commands->size() == COMMANDS);
if (arguments.size() > 0)
{
cmd = commands->begin();
while (cmd != commands->end())
{
if (strcasecmp(cmd->first.command.c_str(),
arguments[0].c_str()) == 0) {
arguments.erase(arguments.begin());
break; // We've got the command
}
cmd++;
}
What's the reason you have a std::map and don't use it for lookup? If
you search for the command string incrementally, you could use a
std::vector instead, or drop the container and search in the
statically declared command_table.
I don't know enough yet to write a find() funtion for the ELEMENT struct
that takes a std::string and returns the appropriate element that the
std::map class can use.
Provide a correct operator<.
I don't know if strcasecmp provides a strikt weak ordering needed for
std::sort and std::map, but std::lexicographic_compare with toupper()
should do it. As fifth parameter to lexicographic_compare, pass a
function that returns toupper(left) < toupper(right).
When that is done, you would have to instantiate an ELEMENT with the
command you want to find and pass it to find(). If find() returns end(),
its not in the map.
In this case, unless you need the struct for something else, I would
drop the ELEMENT struct and use a
std::map<std::string, function_pointer_type, LessComparator>,
where LessComparator would be a function that uses
std::lexicographic_compare internally. You could pass the command you
want to find directly to find().
If the map won't change after initialization, you could even make it a
std::vector, sort it (with LessComparator above), and use
std::binary_search (with same comparator). This saves you all the
overhead of std::map and has better cache locality.
--
Thomas