With this class:
#ifndef __SGI_STL_MAP
#include <map>
#endif
#ifndef __IOSTREAM__
#include <iostream>
#endif
template <typename K,typename V> class HashTable: public std::map<K,V>
{
public:
bool Put(K& k,V& v,bool replace=false)
{
if (replace)
this->erase(k);
return (this->insert(std::make_pair(k,v))) ? true : false;
}
V& Value(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return iter->second;
return NULL;
}
bool Keys(K& k,bool firstp = false)
{
typedef typename std::map<K,V>::iterator map_iter;
static map_iter iter = this->begin();
if (iter == this->end()) return false;
k = iter->first();
++iter;
return true;
}
bool Exists(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return true;
return false;
}
void DumpKeys(void)
{
bool first = true;
K key;
while (this->Keys(key,first))
{
cout << " " << key << endl;
first = false;
}
}
unsigned int EntryCount(void) { return this->size(); }
};
I eliminated 300+ lines of .h and .cxx code
45 lines of code :)
Thanks all who helped :)
___ _ ____ ___ __ __
/ _ )(_) / /_ __ / _ \___ _/ /_/ /____ ___
/ _ / / / / // / / ___/ _ `/ __/ __/ _ \/ _ \
/____/_/_/_/\_, / /_/ \_,_/\__/\__/\___/_//_/
/___/
Texas Instruments ASIC Circuit Design Methodology Group
Dallas, Texas, 214-480-4455, b-******@ti.com 5  983 
On Tue, 7 Dec 2004 13:15:08 -0600, Billy Patton <bp*****@ti.com>
wrote:
[snip] #ifndef __IOSTREAM__
#include <iostream>
#endif
Why? Isn't there already an include guard inside of <iostream>?
(Besides, if this is being compiled on MSVC, it won't help...you need
#pragma once).
template <typename K,typename V> class HashTable: public std::map<K,V>
{
public:
bool Put(K& k,V& v,bool replace=false)
{
if (replace)
this->erase(k);
return (this->insert(std::make_pair(k,v))) ? true : false;
}
No need for superfluous "this" everywhere ...
V& Value(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return iter->second;
return NULL;
}
The same...
bool Keys(K& k,bool firstp = false)
{
typedef typename std::map<K,V>::iterator map_iter;
static map_iter iter = this->begin();
Is this bound to work? "iter" is static, yet you depend on the object
being fully constructed to initialize it ...
if (iter == this->end()) return false;
k = iter->first();
++iter;
return true;
}
The argument firstp is never used...after X runs, where
X==this->size(), the function always returns false ... What is it you
are actually trying to do here??
bool Exists(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return true;
return false;
}
Why not just write:
bool Exists(K& k) { return find(k)!=end(); }
[snip...]
I eliminated 300+ lines of .h and .cxx code
45 lines of code :)
Looks like you aren't quite finished yet ;)
--
Bob Hairgrove No**********@Home.com
Billy Patton wrote: [redacted]
V& Value(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return iter->second;
return NULL;
confusing indentation. Also, NULL cannot be returned. I'm surprised
your compiler allowed it.
}
[redacted]
Billy Patton wrote: With this class:
#ifndef __SGI_STL_MAP
#include <map>
#endif
#ifndef __IOSTREAM__
#include <iostream>
#endif
The include guards are supposed to be in the headers so you don't need
to do that. [does the standard guarantees that?]
template <typename K,typename V> class HashTable: public std::map<K,V>
You are aware that you will not be able to use this class
polymorphically because you inherit from std::map which has no virtual
dtor? I would personally aggregate a std::map, but your design may
require inheritance.
{
public:
bool Put(K& k,V& v,bool replace=false)
{
if (replace)
this->erase(k);
return (this->insert(std::make_pair(k,v))) ? true : false;
}
insert() can't fail, except by throwing an exception.
V& Value(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return iter->second;
return NULL;
That's illegal. You cannot return null since you need a reference.
Throw an exception or change the return type to a pointer.
}
bool Keys(K& k,bool firstp = false)
{
typedef typename std::map<K,V>::iterator map_iter;
static map_iter iter = this->begin();
if (iter == this->end()) return false;
k = iter->first();
++iter;
return true;
}
Maybe the use of a custom iterator would be interesting here.
# include <iostream>
# include <map>
template <class K, class V>
class HashTable;
template <class K, class V>
class HashTable
{
private:
std::map<K, V> map_;
public:
template <class HTK, class HTV>
class key_iterator
{
private:
typedef HashTable<HTK, HTV> HT;
HT &ht_;
typename HT::iterator itor_;
public:
key_iterator(HashTable<HTK, HTV> &ht,
typename HashTable<HTK, HTV>::iterator itor);
K &operator*();
K *operator->();
K &operator++();
K operator++(int);
// ...
};
key_iterator<K, V> key_begin()
{
return key_iterator<K, V>(*this, map_.begin());
}
key_iterator<K, V> key_end()
{
return key_iterator<K, V>(*this, map_.end());
}
};
template <class K, class V>
void f(HashTable<K, V> &ht)
{
std::cout << "Here are the keys: " << std::endl;
for (typename HashTable<K, V>::key_iterator itor=ht.key_begin();
itor!=ht.key_end(); ++itor)
{
std::cout << *itor << std::endl;
}
}
I just wanted to show the iterator's definition, but I got carried :)
bool Exists(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return true;
return false;
}
void DumpKeys(void)
void in the argument list is considered bad style in C++ and is
redundant anyways.
{
bool first = true;
K key;
while (this->Keys(key,first))
{
cout << " " << key << endl;
first = false;
}
}
unsigned int EntryCount(void) { return this->size(); }
Prefer using std::size_t.
};
In general, it's very ok. Here's a couple of things:
1) I usually put a bunch of typedefs
typedef std::map<K, V> map_t;
typedef std::map<K, V>::iterator iterator;
typedef std::map<K, V>::const_iterator const_iterator;
...
at the top of the class so you won't need to do it in every function.
2) Using this-> is redundant in your case.
Jonathan
On Tue, 7 Dec 2004, Bob Hairgrove wrote: On Tue, 7 Dec 2004 13:15:08 -0600, Billy Patton <bp*****@ti.com>
wrote:
[snip] #ifndef __IOSTREAM__
#include <iostream>
#endif
Why? Isn't there already an include guard inside of <iostream>?
Compiling only on Linux and Solaris.
THere is already a guard, but it has to do a file io for it to find the gaurd.
This prevents the file io.
(Besides, if this is being compiled on MSVC, it won't help...you need
#pragma once).
template <typename K,typename V> class HashTable: public std::map<K,V>
{
public:
bool Put(K& k,V& v,bool replace=false)
{
if (replace)
this->erase(k);
return (this->insert(std::make_pair(k,v))) ? true : false;
}
No need for superfluous "this" everywhere ...
V& Value(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return iter->second;
return NULL;
}
The same...
bool Keys(K& k,bool firstp = false)
{
typedef typename std::map<K,V>::iterator map_iter;
static map_iter iter = this->begin();
Is this bound to work? "iter" is static, yet you depend on the object
being fully constructed to initialize it ...
if (iter == this->end()) return false;
k = iter->first();
++iter;
return true;
}
The argument firstp is never used...after X runs, where
X==this->size(), the function always returns false ... What is it you
are actually trying to do here??
bool Exists(K& k)
{
typedef typename std::map<K,V>::iterator map_iter;
map_iter iter = this->find(k);
if( iter != this->end() )
return true;
return false;
}
Why not just write:
bool Exists(K& k) { return find(k)!=end(); }
[snip...]
I eliminated 300+ lines of .h and .cxx code
45 lines of code :)
Looks like you aren't quite finished yet ;)
--
Bob Hairgrove
No**********@Home.com
___ _ ____ ___ __ __
/ _ )(_) / /_ __ / _ \___ _/ /_/ /____ ___
/ _ / / / / // / / ___/ _ `/ __/ __/ _ \/ _ \
/____/_/_/_/\_, / /_/ \_,_/\__/\__/\___/_//_/
/___/
Texas Instruments ASIC Circuit Design Methodology Group
Dallas, Texas, 214-480-4455, b-******@ti.com
Bob Hairgrove wrote: On Tue, 7 Dec 2004 13:15:08 -0600, Billy Patton <bp*****@ti.com>
wrote:
[snip] template <typename K,typename V> class HashTable: public
std::map<K,V>{
public:
bool Put(K& k,V& v,bool replace=false)
{
if (replace)
this->erase(k);
return (this->insert(std::make_pair(k,v))) ? true : false;
}
No need for superfluous "this" everywhere ...
In fact, there is. Older compilers failed to do proper name
lookup, but new compilers correctly require it. The reason is
that tw-phase name lookup looks up "insert" in phase 1
(template compilation, non-dependent name) and "this->insert"
in phase 2 (instantiation, 'this' is dependent name).
In the first phase, the templated base class is unknown,
so std::map<...>::insert won't be found.
Regards,
Michiel Salters This thread has been closed and replies have been disabled. Please start a new discussion. |
