-1

I have a function that returns a Customer object (not pointer) like this:

Customer CustomerList::retrieve(const int index) const{
        if (index<1 || index>size)
                return false;
        else{
                Node *cur = find(index);
                return (cur->data);
        }
}

This function gets a Customer object from a CustomerList(which is a linkedlist).

I'm trying to manipulate the Customer in the list with following function(this function adds an Account to the Customer object.)

list.retrieve(i).addAccount(acc);

However after this function call, Customer object in CustomerList doesn't change. I assume that the reason is I return a copy of a Customer object, not the object itself.

So in order to return the adress of Customer and manipulate it correctly, i make the following changes to my function.

Customer* CustomerList::retrieve(const int index) const{
        if (index<1 || index>size)
                return false;
        else{
                Node *cur = find(index);
                return &(cur->data);
        }
}

And call the manipulating function like that:

list.retrieve(i)->addAccount(acc);

But it gives me a "Access violation reading location 0x00000044." error. What I want to learn is:

  1. Why doesn't it manipulate the Customer object in first place? Is my assumption right?
  2. After I change my functions and function calls, why does it gives me the error I mentioned above?
Burak Özmen
  • 865
  • 2
  • 11
  • 28
  • How are you doing `return false;` when your return type is `Customer`? Does it make sense to convert from a `bool` to a `Customer`? – Joseph Mansfield May 07 '13 at 14:59
  • 1
    have you checked if `cur` is NULL? – Chethan May 07 '13 at 15:00
  • Show us a definition of Customer class – maverik May 07 '13 at 15:00
  • `return false`? Well, that would become zero, although I'm not convinced you can get that to compile. But calling a function from a member that is zero won't work. – Mats Petersson May 07 '13 at 15:00
  • Access violation reading 0x00000044 doesn't mean null pointer deref. Looks like you're trying to dereference a pointer (?) that points to this address (0x00000044) – maverik May 07 '13 at 15:02
  • 3
    @maverik: That just means whatever OP is trying to access is at a 44-byte offset from 0, which I find to be pretty close. – Xeo May 07 '13 at 15:03
  • @Xeo, hm... may be you're right here. Anyway we need to see Node class – maverik May 07 '13 at 15:03
  • @Chethan, I don't think it is related with a NULL check because it is works normally at first state. – Burak Özmen May 07 '13 at 15:05
  • `private: struct Node{ Customer data; Node *next; Node *prev; };` – Burak Özmen May 07 '13 at 15:06
  • 1
    @BurakÖzmen, is it easier to check for NULL rather than guessing why it crashes? – maverik May 07 '13 at 15:06
  • It looks like you are returning `false` which is `0` and then when you call `addAccount` it is accessing a member at an offset `44` bytes from `0` which is what your `this` pointer will be. You need to not return `false` and check the return from `retrieve` is not `NULL` before you use it. This is very similar although not identical to this post http://stackoverflow.com/questions/669742/accessing-class-members-on-a-null-pointer – Shafik Yaghmour May 07 '13 at 15:07
  • Actually it was about NULL check and range check. Never start your checking from 0 when first item has an index of 1. – Burak Özmen May 07 '13 at 15:28

2 Answers2

1

Why doesn't it manipulate the Customer object in first place? Is my assumption right?

As you say, you're returning a copy and manipulating that, leaving the one in the list untouched.

After I change my functions and function calls, why does it gives me the error I mentioned above?

Almost certainly because of this:

return false;

That will return a null pointer if the index is out of bounds. If that's the behaviour you want, then you'll need to check before dereferencing the pointer:

if (Customer * c = list.retrieve(i)) {
    c->addAccount(acc);
} else {
    // handle the error?
}

and, out of politeness, you should return something that looks more like a null pointer such as nullptr, NULL, or 0.

It might be a better idea to throw an exception (perhaps std::range_error); then the caller can assume that the pointer is valid if the function returns. In that case, it might also be better to return a reference rather than a pointer, giving code very much like your original example:

Customer & CustomerList::retrieve(const int index) const{
    if (index<1 || index>size)
            throw std::range_error("Customer index out of range");
    else{
            Node *cur = find(index);
            return (cur->data);
    }
}

list.retrieve(i).addAccount(acc); // Does exactly what you'd expect

I might also consider moving the range checks into the find function, if that seems appropriate.

Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
0
  1. Why doesn't it manipulate the Customer object in first place?

Yes you are right. by defualt its retuned by value not by reference so original object in List is not geting modified.

  1. After I change my functions and function calls, why does it gives me the error I mentioned above?

I think you need to share the code of addAccount Method. the problem may be inside it. Considering then with original code return by value it was working corectly (without exception).

rahul maindargi
  • 5,359
  • 2
  • 16
  • 23