2

This is a snippet of code I use:

void Move::AddToMovesList(Location* &list, int row, int col) {
    // If the list is empty, create the first item
    if (list == NULL)
        list = new Location(row, col);
    // List exists, so append
    else
        list->Add(row, col);
}

If list is NULL, a new Location should be created and the pointer list should point to that new location. That's the behavior I'd expect from this code, but right before gdb exits this function I noticed that list is still NULL. What am I doing wrong here?

I used the ampersand in Location* &list to make sure that I can permanently (vs. locally) change the supplied pointer.

10
  • 4
    Personally I would not use pointer. You are representing the empty list as the NULL pointer. That means you need to add code to explicitly check for this everywhere in your code. Create an object (that initially represents the empty list) then just add members to it no special case required. Commented Oct 1, 2010 at 18:11
  • 1
    Your code should work, did you turn off optimizations when debugging? Commented Oct 1, 2010 at 18:15
  • I'm not sure if I know what you're talking about there, Vincent. I use gdb inside Eclipse with default settings. I'm on Windows and I use the MinGW C++ compiler. Commented Oct 1, 2010 at 18:27
  • 1
    i don't find anything wrong in your code. is list still NULL after returning from the function (i.e., it is still NULL from where you called it)? did you try Location** list and see what happens? Commented Oct 1, 2010 at 18:34
  • 1
    I think it will get much easier to think about once you fix your class names. The word Location is singular, reading it does not make one think of a list. Passing multiple arguments to a list constructor does not make one think that they will all be used to initialize one element and the resulting list will have length one. Commented Oct 1, 2010 at 19:04

2 Answers 2

1

Let's not reinvent the wheel here... Know your libraries.

If you use the STL list container (or any other container) then you don't need to bother with null pointers.

Sign up to request clarification or add additional context in comments.

1 Comment

I'm doing this for a school project and we've not covered lists in our C++ course so far, so unfortunately I'm not allowed to use them.
0

Personally I would not use pointer at all.

class Location
{
    public:
        void add(int row, int col)
        {
            data.push_back(std::make_pair(row,col));
        }
        bool isEmpty()  const {return data.empty(); }
    private:
        std::vector<std::pair<int,int> > data;
};


class Move
{
    public:
        // Pass a reference to the list.
        // No special case processing if the list is empty.
        // No problems with ownership.
        // No problems with lifespan associated with new/delete
        void addToMoveList(Location& list, int row, int col)
        {
            list.add(row, col);
        }
};

int main()
{
    Location    list;  // Don't use new if you can declare a local variable.
    Move        move;

    move.addToMoveList(list, 10, 2);
}

2 Comments

Looks much easier to implement, but as I mentioned in another comment I'm doing a school project on C++ and we've hardly covered any C++-specific advantages. I'm not allowed to use stuff we haven't covered in the project, so for now I think I have to stick to the C alternative of creating dynamically linked lists.
@Pieter - you may not be allowed to use STL etc, but still you don't have to do it in C fashion, with brute-force memory management via new/delete/malloc/free and raw pointers everywhere.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.