Re: Problems with std::map.insert() and std::map.find()
On Dec 20, 3:45 am, "(2b|!2b)==?" <void-s...@ursa-major.com> wrote:
I have written a class template to store manage/raw pointers,
I want to store pointers to an obects, using a key. I find
that find() is not working (after the first insert) - i.e.
once the first item is inserted in the map, any key passed to
the find() method will return the first item.
Additionally, once the first item has been stored, no
additional items can be stored - its driving me nuts, and I
cant work outt why this is happening. I have included both my
classes (i.e class template and key class here - hopefully,
someone can spot what is causing things to go awry). To
summarize the problem - every - i.e. find() and insert() work
as expected, UNTIL the first item is inserted into the map -
which seems to suggest that somehow, the Keys are considered
non-unique???
The code follows below:
PointerMap.h
=============
#pragma once
#include <string>
#include <map>
#include <stdexcept>
template<class T1, class T2>
class PointerMap
{
public:
typedef typename std::map<T1, T2*>::reference reference;
typedef typename std::map<T1, T2*>::const_reference const_referen=
ce;
typedef typename std::map<T1, T2*>::const_iterator const_iterator=
;
typedef typename std::map<T1, T2*>::iterator iterator;
PointerMap()
{
}
~PointerMap()
{
clear();
}
iterator begin() { return m_map.begin(); } iterator
end() { return m_map.end(); } const_iterator begin()
const { return m_map.begin(); } const_iterator end()
const { return m_map.end(); } iterator find(T1 key) {
return m_map.find(key) ; } const_iterator find(T1 key)
const { return m_map.find(key) ; }
iterator erase(iterator where)
{
if(where != m_map.end() && *where)
{
T*& ptr = *where;
delete ptr;
ptr = 0;
}
return m_map.erase(where);
}
iterator erase(iterator begin, iterator end)
{
while(begin != end)
{
T2*& ptr = begin->second ;
if ( ptr != 0)
{
delete ptr ;
ptr = 0;
}
begin++;
}
return m_map.erase(begin, end);
}
void clear() { erase(begin(), end()); }
bool empty() const { return m_map.empty(); }
size_t size() const { return m_map.size(); }
void insert(T1 key, T2* val)
{
m_map.insert(std::pair<T1, T2*>(key, val));
}
This is (generally) inconsistent with your erase function.
Personally, I question the utility of such a class to begin
with; I've never had a case where a map should delete a pointer
when it is removed from the map. But if it's going to make
sense, then either the map has to also allocate the memory, or
there must be a clear transfer of responsibility, for example by
insert taking an std::auto_ptr. And some very, very rigorous
documentation, to the effect that erase may invalidate pointers
in user code.
T2* operator[] (const T1 idx) const
{
if (idx > m_map.size())
throw std::logic_error("Array bounds exceeded");
This doesn't make sense. What if T isn't comparable with an
integral type (the usual case when map is being used)?
else
return m_map[idx] ;
And this isn't legal: you can't invoke [] on a constant map.
}
If you want to support [] on a constant map (which makes a lot
of sense if the return value is a pointer), then something like:
T2* operator[]( T1 idx ) cosnt
// ^^ no real point in the const here...
{
const_iterator result = m_map.find( idx ) ;
return result == m_map.end()
? NULL
: result->second ;
}
T2*& operator[] (const T1 idx)
{
if (idx > m_map.size())
throw std::logic_error("Array bounds exceeded");
Same comment as above.
else
{
T2*& ptr = m_map[idx] ;
Note that this inserts an element, mapping to a null pointer, if
one isn't already present. You may prefer doing something like
I suggested for the const [], to avoid this.
if (ptr)
{
delete ptr ;
ptr = 0 ;
}
return m_map[idx] ;
In otherwords, regardless of the index, you return a null
pointer:-). And if there was something there, you delete it.
}
}
I think you have to decide on one policy for [], and stick to
it. The standard uses the policy that if the element isn't
there, it gets inserted, with the mapped to entry being
constructed with a default constructor (for pointers, set to
null). That can be a useful policy in some cases, but not
necessarily everywhere. And it introduces two important
restrictions: you can't use [] on a const map (since it may
modify the map), and if you use [], the instantiation type must
support default construction.
Another frequent policy is to provide a contains(key) function,
and make it a pre-condition for [].
Or you can return some sort of "fallible" object, with a status
which indicates whether the object was found or not. In the
case of a map, the simplest way to do this is to return a
pointer to an element, rather than the element itself, returning
a null pointer if the element didn't exist. If the map
maintains an invariant which excludes some specific values (e.g.
a pointer map which doesn't allow null pointer entries), then
you can return one of the excluded values (a null pointer).
Otherwise, you can use a true Fallible class (but in this case,
I find the pointer to the element a better solution---the
Fallible class is mainly useful when you have to return a
value.)
private:
PointerMap(const PointerMap& source);
PointerMap& operator=(const PointerMap& source);
std::map<T1, T2*> m_map;
};
RepositoryKey.h
================
class RepositoryKey
{
public:
RepositoryKey(const unsigned char xch, const long ic, const
std::string& syb, const long fq = 0):
m_xch(xch), m_ic(ic), m_syb(syb), m_fq(fq)
{
}
RepositoryKey(const RepositoryKey& key):
m_xch(key.m_xch), m_syb(key.m_syb), m_ic(key.m_ic), m_fq(=
key.m_fq)
{
}
~RepositoryKey()
{
}
RepositoryKey& operator=(const RepositoryKey& key)
{
if (this != &key)
{
m_xch = key.m_xch;
m_syb = key.m_syb;
m_ic = key.m_ic ;
m_fq = key.m_fq ;
}
return *this;
}
bool operator==(const RepositoryKey& key) const
{
return ( (m_xch == key.m_xch) && (m_syb == key.m_=
syb) &&
(m_ic == key.m_ic) && ((long)m_fq =
== (long)key.m_fq) );
}
bool operator < (const RepositoryKey& key) const
{
return ( (m_xch < key.m_xch) && (m_ic < key.m_ic) && ((lo=
ng)m_fq <
(long)key.m_fq)
&& (_stricmp(m_syb.c_str(), key.m_syb.c_s=
tr()) < 0));
}
This doesn't establish a partial order. For a partial order,
a<b || b<a is always true, a<b && b<a implies equality, and (a<b
&& b<c) == a<c. Try the last with a==(1,2...), b==(2,1...) and
c==(0,2...): a<b, b<c and c<a.
The canonlical pseudo-code for a comparison operator over n
fields would be something like:
bool
lessThan<n>( Type const& lhs, Type const& rhs )
{
return lhs.field<n> < rhs.field<n>
|| (! rhs.field<n> < lhs.field<n>
&& lessThan<n+1>( lhs, rhs ) ;
// except when n is the last field, of
// course.
}
bool
operator<( Type const& lhs, Type const& rhs )
{
return lessThan<0>( lhs, rhs ) ;
}
(Note that this is pseudo-code, and the <n> notation shouldn't
be meant to imply templates. Although with the upcoming
standard, and variadic template arguments, it is probably
possible to implement such a template. Otherwise, you actually
have to manually implement all of the lessThan<n> functions.)
If your type supports ==, as well as <, then it is sometimes
preferable to replace the (! rhs.field<n> < lhs.field<n>) with
rhs.field<n> != lhs.field<n>. For a lot of types, it may even
be interesting to provide a single compare function, which
returns a value <, == or > 0, depending on the results of the
comparison. In that case, you get:
bool
lessThan<n>( Type const& lhs, Type const& rhs )
{
int result
= compare( lhs.field<n>, rhs.field<n> ) ;
return result < 0
|| (result == 0 && lessThan<n+1>( lhs, rhs )) ;
}
In a very real sense, think of it in the same way you would
compare strings, character by character, except that the fields
are your 'characters'.
--
James Kanze (GABI Software) email:james.kanze@gmail.com
Conseils en informatique orient=E9e objet/
Beratung in objektorientierter Datenverarbeitung
9 place S=E9mard, 78210 St.-Cyr-l'=C9cole, France, +33 (0)1 30 23 00 34