-1

Hi I'm a student developing various abstract data types for learning purposes. I am trying to overload the subscript operator to look up the keys in these ADT's. I cannot avoid the case where my code inserts missing keys.

It is intended that when reading from these ADT's a return value of nullptr will indicate that the key was not found. I want my ADT's to add new keys only when writing or inserting. My code is incorrect in how it handles the following:

const char *x = tree["key"];

Here if "key" is not found I wish to avoid adding this key and return nullptr to indicate its absence. The key should only be added in the following situation:

tree["key"] = "x";

This is a very subtle bug. The following test returns true, but only because the value being tested is hash["key"].val (which happens to be null).

if (!x) printf("%s not found!\n", "key");

I believe this can be achieved through the use of const and have used the following signature.

char const *operator[](char *index) const;

This const overload is never invoked. Can somebody explain why this is the case and either an example signature which will behave properly (or an example which will force this overload to be evaluated). Here is my code:

#include <stdio.h>
#include <string.h>

class lookup {
    lookup *left;
    lookup *right;
    char *key;
    char *val;

public:
    lookup(char *k) {
        left = nullptr;
        right = nullptr;
        val = nullptr;
        key = new char[strlen(k)+1];
        strcpy(key, k);
    }

    ~lookup() {
        if (key) delete key;
        if (val) delete val;
    }

    /* read/write access */
    /* if the key does not exist, then create it */
    char *&operator[](char *index) {
        printf(" []= x\n");

        int x = strcmp(index, key);
        if (x < 0) {
            if (left == nullptr) left = new lookup(index);
            return (*left)[index];
        }
        if (x > 0) {
            if (right == nullptr) right = new lookup(index);
            return (*right)[index];
        }
        return val;
    }

    /* read only access */
    /* if the key does not exist, then return nullptr (not found) */
    char const *operator[](char *index) const {
        printf(" x = *[]\n");

        int x = strcmp(index, key);
        if (x < 0) {
            if (left == nullptr) return nullptr;
            else return (*left)[index];
        }
        if (x > 0) {
            if (right == nullptr) return nullptr;
            else return (*right)[index];
        }
        return val;
    }
};

int main(void) {
    lookup tree("root");

    /* this creates the key (which is not intended) */
    const char *x = tree["key"];
    if (!x) printf("%s not found!\n", "key");

    /* only now should the key be created */
    tree["key"] = "value";
    return 0;
}
clns
  • 1
  • Note that if you allocate something with `new[]` you must free it with `delete[]`. And your code would be a lot clearer and simpler if you used std::strings instead of C-style strings. –  Jul 19 '17 at 22:52
  • `if (key) delete key;` -- Wrong form of `delete`. It should be `delete [ ]`. Avoid this and just use `std::string`. – PaulMcKenzie Jul 19 '17 at 22:55
  • *This const overload is never invoked.* -- Pass `lookup` as a const reference to a function and try to invoke `lookup::operator[ ]` inside that function. The compile has no choice but to call the `const` version. – PaulMcKenzie Jul 19 '17 at 22:58
  • Also note that `std::map::operator[ ]` inserts a value if it doesn't exist. That's why you don't call it if you don't want that behavior, and functions such as `map::insert()` exist. Why not design your class in a similar fashion? – PaulMcKenzie Jul 19 '17 at 23:04
  • @PaulMcKenzie I don't see how `std::map` is relevant here. It is not used. – eerorika Jul 19 '17 at 23:05
  • @user2079303 -- My point is that the OP's design could mimic what `std::map` does. There is no need for extraneous `operator [] ` behavior -- just create an `insert()` function that actually checks for duplicates. – PaulMcKenzie Jul 19 '17 at 23:06
  • Thanks all. That pass as const trick fixed it Paul. I will look into the maps once I get more familiar with them. – clns Jul 19 '17 at 23:08
  • @PaulMcKenzie ah, now I see what you mean. Although I think it's the `map::find` that's what clns is missing. – eerorika Jul 19 '17 at 23:18

1 Answers1

1

You want tree["key"] to behave differently in const char *x = tree["key"]; than in tree["key"] = "x";. That is just not possible. The position of the sub expression in the statement has no effect on how that sub expression behaves.

You might achieve behaviour similar to what you're looking for if you can change the return type of the subscript operator. If you never add the missing value to the tree, and return some object whose behaviour is to add the element (if missing) on assignment, then you can use have the above two statements to behave as you like.

That said, the above may be tricky to implement and there may be caveats. Perhaps you don't need to be able to use operator[] to look up an object without inserting on miss. Instead of trying to use an overload, it would be simpler to use a separate function to do it, like std::map which Paul suggested.

eerorika
  • 232,697
  • 12
  • 197
  • 326