Compile errors using find method with a map

I could use String objects with this, but this code has to interact with fortran and C code through an RDMA transport on Jaguar (Titan), so serializing and sending complex objects isn't possible (certainly possible for me to use internally, though).

Basically, I'm trying to maintain a map of transactions, identified by name. However, I can't seem to use the "find" method on my map. Here's an example of what I'm trying to do (real code is export controlled, so this is a good mimic):

Code:

#include <map>
#include <stdio.h>

#define TRANSACTION_NAME_SIZE 32

struct _txn_string
{
    char name[TRANSACTION_NAME_SIZE];
};
typedef struct _txn_string txn_string;

struct _mpi_sub_tx
{
    char name[TRANSACTION_NAME_SIZE];
    int num_operations;
};
typedef struct _mpi_sub_tx mpi_sub_tx;

class compare_txn_string{
public:
    bool operator()(const txn_string * str1, const txn_string * str2)
        {
            return strncmp(str1->name, str2->name, TRANSACTION_NAME_SIZE);
        }
};

using namespace std;

int main()
{
    map<txn_string*, mpi_sub_tx*> buckets(compare_txn_string());
    txn_string * str1 = new txn_string();
    mpi_sub_tx * sub1 = new mpi_sub_tx();
    strcpy(str1->name, "hello");
    strncpy(sub1->name, str1->name, TRANSACTION_NAME_SIZE);
    printf("str name: %s sub1 name: %s\n", str1->name, sub1->name);

    if(buckets.find(str1) == buckets.end())
        buckets[str1] = sub1;
    return 0;
}

The compile error is:

Code:

g++ -o stupid stupid.cpp
stupid.cpp: In function ‘int main()’:
stupid.cpp:38: error: request for member ‘find’ in ‘buckets’, which is of non-class type ‘std::map<txn_string*, mpi_sub_tx*, std::less<txn_string*>, std::allocator<std::pair<txn_string* const, mpi_sub_tx*> > > ()(compare_txn_string (*)())’
stupid.cpp:38: error: request for member ‘end’ in ‘buckets’, which is of non-class type ‘std::map<txn_string*, mpi_sub_tx*, std::less<txn_string*>, std::allocator<std::pair<txn_string* const, mpi_sub_tx*> > > ()(compare_txn_string (*)())’
stupid.cpp:39: error: invalid types ‘std::map<txn_string*, mpi_sub_tx*, std::less<txn_string*>, std::allocator<std::pair<txn_string* const, mpi_sub_tx*> > > ()(compare_txn_string (*)())[txn_string*]’ for array subscript

Ideally, the if statement should be true as I haven't entered anything in the map yet...


Any ideas what my misconception is?

Okay, I got it. The way I was doing the buckets before was wrong.

I changed it to

Code:

    map<txn_string*, mpi_sub_tx*, compare_txn_string> buckets;

Are you sure you have your compare class correct. The strncmp() returns an int, not a bool and this int can have three different values, 1,0,-1. So your class is returns false when the strings match, but true any other time. This doesn't seem like a less than comparison to me.

Jim

Originally Posted by jimblumberg

Are you sure you have your compare class correct. The strncmp() returns an int, not a bool and this int can have three different values, 1,0,-1. So your class is returns false when the strings match, but true any other time. This doesn't seem like a less than comparison to me.

Jim

Interesting. The code seems to work perfectly with the slight modification I've made in my 2nd reply..... You SHOULD be right, but I can't understand why you aren't....

the code may "work" as is, but a std::map tries to order its elements on insertion. if you don't care about the order, your method will work just fine. the way you have written it, the map will (probably - I haven't tested it) always insert the new elements at the beginning, unless the key is equal to an existing element's key.

Originally Posted by dayalsoap

The code seems to work perfectly with the slight modification I've made in my 2nd reply..... You SHOULD be right, but I can't understand why you aren't....

Did you test after populating the std::map with many elements of various values? Breaking the pre-condition that the comparator implement a "less than" comparison results in undefined behaviour. Yet, it is so incredibly easy to fix:

Code:

return strncmp(str1->name, str2->name, TRANSACTION_NAME_SIZE) < 0;

Originally Posted by Elkvis

the code may "work" as is, but a std::map tries to order its elements on insertion. if you don't care about the order, your method will work just fine.

The problem is, the ordering established by the std::map is used to find the key. This is kind of like trying to perform binary search on an unsorted array.

Originally Posted by dayalsoap

Interesting. The code seems to work perfectly with the slight modification I've made in my 2nd reply..... You SHOULD be right, but I can't understand why you aren't....

You mean that you cant understand why you think it's okay as-is.

The answer is that jimblumberg is right. But you wont see the problem unless you try the things that will fall over.
If you insert say ten items in descending order, then you will discover that you cannot find all ten items afterwards.
It must be done as Laserlight shows.

You are right.

With repsect to Laserlight, why are we checking for < 0? Shouldn't the condition be != 0?

Thanks again.

Originally Posted by iMalc

You mean that you cant understand why you think it's okay as-is.

The answer is that jimblumberg is right. But you wont see the problem unless you try the things that will fall over.
If you insert say ten items in descending order, then you will discover that you cannot find all ten items afterwards.
It must be done as Laserlight shows.

Originally Posted by dayalsoap

With repsect to Laserlight, why are we checking for < 0? Shouldn't the condition be != 0?

Because the comparison is a "less than" comparison (or "strict weak ordering", if you prefer mathematical jargon).