0

So I'd want to use a Set of char[4] so that I can store such arrays in it and check if an element is already in there. I began with C++ today so I definitely am not an expert. Here is what I've done for the moment :

struct Comparator {
    bool operator()(unsigned char* s1, unsigned char* s2) const {
        for (int i = 0; i < 4; ++i) {
            if (s1[i] != s2[i]) {
                return s1[i] < s2[i];
            }
        }
        return false;
    }
};

int main(int argc, const char* argv[]) {
    set<unsigned char*, Comparator> setValues;
    unsigned char *v1 = (unsigned char*) malloc(4);
    unsigned char *v2 = (unsigned char*) malloc(4);
    v1[0] = 'a';
    v1[1] = 'b';
    v1[2] = 'c';
    v1[3] = 'd';

    v1[0] = 'a';
    v1[1] = 'b';
    v1[2] = 'c';
    v1[3] = 'd';

    setValues.insert(v2);

    printf("%lu\n", setValues.count(v1));
    free(v1);
    free(v2);
    return EXIT_SUCCESS;
}

Unfortunately the output of this program is "0". Given that the chars between v1 and v2 are the same, I hoped that the output would be "1" (I inserted v2 and tried to find v1).

How can I do so that the set compares the values of the arrays (and not the pointer as it seems to do) ? I've searched and tried to overload operator "==" without success, and people seem to say that overloading "()" is enough...

Another thing : I'm also really not sur about the set allocation. I know that my set will have 100 arrays in it at the end of my program execution so I wonder if there is a way to directly allocate 100 * char[4] for the set ?

Thank you very much!

Jo8192
  • 19
  • 2
  • 1
    First advice : __do not mix C and C++__ as most as you can. Do not use `malloc` but `new` and even if you stick with malloc, [do not cast its result](http://stackoverflow.com/a/605858/3460805). Second advice, simple use `std::string` to store characters. A task like yours does not need manual allocation at all. Let the standard containers do it for you. – Chnossos May 02 '14 at 17:13
  • Style points and good practice aside, here is what you need to know to understand what's going on: First, you need to change your comparator as in the [answer](http://stackoverflow.com/a/23433574/3100771). Next, you initialize `v1` twice, and never `v2`; look at the left hand side of your 'a','b','c','d' assignments. Since you ever initialize `v2` you're just inserting the array with random garbage memory, and when you search for `v1` the garbage value of `v2` you inserted will of course not be found. Change these two things and your program will function as you expected. – Apriori May 02 '14 at 18:00
  • Actually I can't convert all my char[4] to str (char[5]) because I have to optimize my code as much as possible.. Thanks for your advice concerning malloc & cie :) – Jo8192 May 02 '14 at 18:08
  • About allocation: Look at the 3rd template parameter to `set`. You can make your own STL `allocator` object to pass here to handle allocation. It's somewhat of an advanced topic, but if you want to be able to pre-allocate 100 elements using a set this is how you'd do it; that is the pre-allocation would happen inside the `allocator` object and be passed to the `set` when the `set` asks for the memory internally. This answer: http://stackoverflow.com/questions/1303993/pre-allocate-space-for-c-stl-queue show's how to do this using Boost (Boost has an `allocator` object you can use). – Apriori May 02 '14 at 18:24
  • 1
    Let me get this straight. You started with C++ today, and you are already worried about optimization issues because of 1 extra character in the arrays? Doesn't make sense if you ask me. – shawn1874 May 02 '14 at 22:07
  • @Chnossos, the result of malloc must be cast in C++ as C++ lacks implicit conversion from `void *` – M.M May 03 '14 at 01:54

1 Answers1

1

There is an error within your example. v2 was never initialized. v1 is set twice. Therefore you are putting in a garbage value into the set.

shawn1874
  • 1,288
  • 1
  • 10
  • 26