not compiling...

Code:

---

#include <iostream>
#include <list>
#include <algorithm>
#include <set>
#include <iterator>

using namespace std;

int main()
{   
      set<string> words;
      list<string> index;
     
      for(string new_word; cin >> new_word, new_word != "Quit"; ) {
          pair<set<string>::iterator, bool> &trace = words.insert(new_word);
          if(trace.second)
              index.push_back(trace.first);
      }
      copy(index.begin(), index.end(), ostream_iterator<string>(cout, "\n"));
      copy(words.begin(), words.end(), ostream_iterator<string>(cout, "\n"));
     
      return 0;
}

---

In line
pair<set<string>::iterator, bool> &trace = words.insert(new_word);

it says

invalid initialization of non-const reference of type 'std::pair<std::_Rb_tree_const_iterator<std::strin g>, bool>&' from a temporary of type 'std::pair<std::_Rb_tree_const_iterator<std::strin g>, bool>'

I took the code from a book so i'm confused on why it won't work...

Here's my guess: notice the "non-const" -- this is a reference, think about why the const is important with that,* so:

Code:

---

      for(string new_word; cin >> new_word, new_word != "Quit"; ) {
          const pair<set<string>::iterator, bool> &trace = words.insert(new_word);
          if(trace.second)
              index.push_back(*(trace.first));
      }

---

I also de-reference the *(iterator) here to yield an actual string (and not a string::iterator).

Compiles fine!

* C++ objects (unless created with new) are "auto-destroyed" based on their reference count (and a reference counts :p); if the reference counting does not track re-assignments (only scope, etc), we cannot permit this reference to be re-assigned or the count will be wrong, resulting in leaked memory. So the compiler says this must be const (sort of like some interior pointers used in C standard lib functions). Nb, this one is destroyed by scope and flow anyway.

Set's insert does not return a reference to a pair.

Quote:

---

Originally Posted by whiteflags

Set's insert does not return a reference to a pair.

---

Yeah, but that will work anyway -- you just need the const.

Of course, it is pointless in context (using a reference).

Quote:

---

Originally Posted by MK27

* C++ objects (unless created with new) are "auto-destroyed" based on their reference count (and a reference counts :p); if the reference counting does not track re-assignments (only scope, etc), we cannot permit this reference to be re-assigned or the count will be wrong, resulting in leaked memory. So the compiler says this must be const (sort of like some interior pointers used in C standard lib functions). Nb, this one is destroyed by scope and flow anyway.

---

No idea what you mean by this.
The reason you cannot assign it a non-const reference is to stop you from accidentally shooting yourself in the foot. The changes in a temporary will not be reflected in the caller (because it's a temporary).
But the standard allows you to bind temporaries to const references, in which case the lifetime of the temporary will be extended.
Also, C++0x also allows you to bind temporaries to r-value references.

I'm still confused. In the first place, how is a return value a "temporary"?

Why does what C++0x let you do matter right now? Is insert's return value an r-value? No.

Of course a return value is a temporary. It's a copy (unless it actually returns a reference) of whatever value the function deemed fit to return. Furthermore, it's an r-value, because it cannot be assigned (again, if it doesn't return a reference).
In other words, if...
words.insert(new_word) = something;
...works, then it's an l-value and can be assigned to a non-const reference.
If it doesn't work, then it's an r-value, and that means it can only be assigned to const references.

And can it matter? Yes. Why? Because a copy is made once you return the value and a copy is made when you assign the value.
Unless, that is, the compiler supports named return value optimization.
If you bind the return value to an r-value reference, you can just steal the temporaries contents without having to allocate more resources and copy everything over and later destroying the temporaries resources.

Well... the original code was this:

Code:

---

#include <iostream>
#include <iterator>
#include <list>
#include <algorithm>
#include <set>

using namespace std;

int main()
{   
      typedef set<string> WordSet;
      typedef WordSet::iterator WordIter;
      typedef list<WordIter> Index;
     
      WordSet words;
      Index input_order;
     
      for(string new_word; cin >> new_word && new_word != "Quit";) {
          const pair<WordIter, bool> &ref = words.insert(new_word);
          if(ref.second)
            input_order.push_back(ref.first);
      }
      copy(input_order.begin(), input_order.end(), ostream_iterator<string>(cout, "\n"));
      copy(words.begin(), words.end(), ostream_iterator<string>(cout, "\n"));
     
      return 0;
}

---

And it shows some crazy long error at the bolded line... i mean why did they put this code if this doesnt work? Weird...

That is a very good question. I have honestly no idea why they did it. The problem is that input_order stores iterators to strings. Hence the bolded copy line sends iterators to an output iterator that is expecting a string.

I was able to get it compiling by...

Code:

---

typedef list<WordIter> Index;
to
typedef list<string> Index;

input_order.push_back(ref.first);
to
input_order.push_back(*ref.first);

---

Another solution might be (more efficient):

Code:

---

Replace
copy(input_order.begin(), input_order.end(), ostream_iterator<string>(cout, "\n"));
by
for_each(input_order.begin(), input_order.end(), [](WordIter it) { std::cout << *it << "\n"; });

---

(Requires C++0x.)