Cannot dereference end list iterator

Question

I'm currently working on a project about browser histories. I'm using the STL's list implementation to keep track of the different websites. I have most of it figured out but I can't seem to resolve the error:

Screenshot; cannot dereference end list iterator.

Screenshot; cannot dereference end list iterator.

The website data is contained within my Site objects.

class BrowserHistory {
   private:
      list<Site> history;
      list<Site>::iterator current = history.begin();
   public:
      void visitSite(string, size_t)
      void backButton();
      void forwardButton();
      void readFile(string);
};

void BrowserHistory::visitSite(string x, size_t y)
{
   while (current != history.end()) {
       history.pop_back();
   }
   history.push_back({ x, y });
   current++;
}

void BrowserHistory::backButton()
{
   if (current != history.begin())
       current--;
}

void BrowserHistory::forwardButton()
{
   if (current != history.end())
       current++;
}

void BrowserHistory::readFile(string filename)
{
   string action, url;
   size_t pageSize;
   ifstream dataIn;
   dataIn.open(filename);

   while (!dataIn.eof()) {
       dataIn >> action;
       if (action == "visit") {
           dataIn >> url >> pageSize;
           history.push_back({ url, pageSize });
           current++;
       }
       if (action == "back") {
           current--;
       }
       if (action == "forward") {
           current++;
       }
   }
   dataIn.close();
}

Can anyone explain to me what's wrong? Thanks in advance for any help.

Welcome on SO! Please post the exact error you get. Also explain what attempts you have made to solve the problem. Btw, your code does not appear to contain any de-referencing of an iterator. — Walter, Oct 17 '19 at 07:21
You should run your program in the debugger and examine the program state at the point where you get this error so you can figure out how the program got into that state ("Why is the iterator at the end?") or whether some piece of code is making an incorrect assumption ("I didn't account for the possibility that the iterator is at the end here."). — chris, Oct 17 '19 at 07:22
Probably unrelated, but you should give [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) a read-through. — user4581301, Oct 17 '19 at 07:24
Your code is very error prone, as it never checks whether `current` is a valid iterator and/or whether the in- and de-crement operators are possible (you shouldn't de-reference or increment the end iterator, you shouldn't decrement the begin iterator), mind you that for an empty list begin=end. — Walter, Oct 17 '19 at 07:27
`push_back` can invalidate you stored iterator. You can easily replace the iterator with an index. — churill, Oct 17 '19 at 07:31
In `BrowserHistory::visitSite()` you pop the last entry if this is the current entry? this does not make sense from the name of the function. In `BrowserHistory::readFile()` you should re-use the functions `visitSite`, `backButton` and `forwardButton`. — Walter, Oct 17 '19 at 07:32
If `current` is a valid iterator, no amount of `pop_back` operations will turn `current` into the end iterator. Use `history.erase(current + 1, history.end())` instead. — Botje, Oct 17 '19 at 07:34
@Walter quoting from [cppreference](https://en.cppreference.com/w/cpp/container/vector/push_back) "If the new size() is greater than capacity() then all iterators and references (including the past-the-end iterator) are invalidated. Otherwise only the past-the-end iterator is invalidated." — churill, Oct 17 '19 at 07:44
@churill I think you confused `std::list` with `std::vector`. Read the post more carefully before commeting. — Walter, Oct 17 '19 at 18:00

molbdnilo · Answer 1 · 2019-10-17T08:39:04.027

When you initialise current, the list is empty.
Adding elements to the list does not suddenly make it a valid iterator, even though it will be different from end.

It looks like you're thinking of iterators as very much like pointers, but they're not.

Iterators are for iterating and should be considered transient, and not stored for later use.

Use a vector, and use an index for current instead of an iterator.

Further, I would rename the "button" functions (what does the history care about buttons?) to "goBack" and "goForward" and use your actual interface when reading:

void BrowserHistory::readFile(string filename)
{
   ifstream dataIn(filename);
   string action;

   while (dataIn >> action) {
       if (action == "visit") {
           string url;
           size_t pageSize;
           if (dataIn >> url >> pageSize) {
               visitSite(url, pageSize);
           }
           else {
               // Handle error
           }
       }
       else if (action == "back") {
           goBack();
       }
       else if (action == "forward") {
           goForward();
       }
       else {
           // Handle error
       }
   }
}

you edited to implement my suggestions (among other things). Btw, shouldn't if be `goBackward` and `goForward` (for semantic symmetry)? — Walter, Oct 17 '19 at 18:03
@Walter I stuck close to the original, but yes. Or "goBackwards" and "goForwards". — molbdnilo, Oct 18 '19 at 06:21

Cannot dereference end list iterator

1 Answers1