| re: problems with stl map iterators
Christian Christmann wrote:[color=blue]
> Hi,
>
> I've some problems with iterators on hashtbales.[/color]
You're not kidding - although they're maps, not hash tables.
[color=blue]
> My hashtable.h
>
> class HashTable
> {
> map<int, void*> nhash;
> map<int, void*> getNhash();
> ...
> }[/color]
You seem to be returning the nhash member by value here, which is
unusual to say the least. This forces a copy to be made each time
getNhash() is called.
As an aside, I'm assuming this summary is slightly wrong, since both
nhash and getNhash() are private ...
[color=blue]
> ----------
>
> My hashtable.cpp:
>
> map<int, void*> HashTable::getNhash() {
> return nhash;
> }[/color]
I'm just going to remove the extra closing curly bracket, but in
general
it is nice if the posted code actually compiles (since you're not
asking
about a compiler error, at least).
[color=blue]
> void HashTable::Insert(int ky,void* entr) {
> nhash.insert(pair<int, void*>(ky, entr));
> }
> ...
>
> ----------
>
> In another class I use this hashtable:
>
> HashTable lNodes;
> const std::map<int, void*>::const_iterator begin =
> lNodes.getNhash().begin(); const std::map<int, void*>::const_iterator[/color]
end[color=blue]
> = lNodes.getNhash().end(); std::map<int, void*>::const_iterator iter[/color]
=[color=blue]
> begin;[/color]
Mmm, unreadable goo. With permission, I'm going to reformat that so I
have room to hang my comments.
typedef std::map<int,void*>::const_iterator HashIter;
HashIter begin = lNodes.getNhash().begin();
// make a temporary copy of lNodes' std::map, get the begin iterator
// from it, and then throw that copy away. 'begin' is now an
iterator
// into a destroyed temporary copy of a map.
HashIter end = lNodes.getNhash().end();
// make another temporary copy of lNodes' std::map, get the end
// iterator from this copy, and then throw the copy away. 'end' is
// now an iterator into another destroyed temporary copy of a map.
HashIter iter = begin;
// copy of an iterator pointing to a destroyed temporary map[color=blue]
>
> cout << "Size: " << lNodes.getNhash().size() << endl;[/color]
This creates the third temporary copy of lNodes.nhash so far, but at
least it should give the right result.
[color=blue]
> while (iter != end)
> {
> cout << "Key: " << iter->first << endl; ++iter;[/color]
As soon as you dereference an iterator to a deleted map, you're in
'undefined' territory. Simply getting the wrong result is pretty much
the least catastrophic outcome you could hope for.
[color=blue]
> }
> }
>
>
> The output:[/color]
<snip - unsurprisingly wrong output>
[color=blue]
> There are always some elements missing. Even if the size is 3 just 1
> element is printed.
> Only with hashtables of size 1 I get the correct output.
>
> What's wrong?[/color]
In case you haven't spotted it from the comments above yet, the problem
is that you're making temporary copies of the map and then throwing
them
away. These anonymous temporaries get destroyed as soon as you hit the
';' at the end of the statement where they're created, ie, at the end
of
"... lNodes.getNhash() ... ;"
If you want to get the size of, and iterators into, an HashTable's
nhash
member rather than making a copy and using that, you need getNhash() to
return a reference rather than a value. Change
[color=blue]
> map<int, void*> getNhash();
> map<int, void*> HashTable::getNhash()
> {
> return nhash;
> }[/color]
to
map<int, void*>& getNhash();
map<int, void*>& HashTable::getNhash()
{
return nhash;
}
and everything will work as you expect. No other changes required.
Especially if you're coming from a language (like Java or Python) that
always passes by reference instead of value [1]: remember that C++
allows you to specify whether your arguments and return values have
reference or value semantics. If you want reference semantics, you
need
to put the '&' in to indicate it.
[color=blue]
> Thank you for your help[/color]
np, hth.
[color=blue]
> Chris[/color]
[1] - ok, Java doesn't always pass by reference, just mostly.
| 
