From: Joseph M. Newcomer on
Now that's a different question. Since you are storing a structure, you would have to
provide your own hashing function. Note the comment "works for most primitive values".
Your SDataType is not a primitive value, and consequently this hash function is not
sufficient.
joe

On 2 May 2007 21:14:27 -0700, Vahid <vahid.ha(a)gmail.com> wrote:

>Thanks for your reply Joseph.
>
>I have already tried a few combinations such as the one you suggested.
>
>Yours will create the following errors:
>
>error C2440: 'type cast' : cannot convert from 'SDataType' to
>'DWORD_PTR' (refers to the "return" line in
>
>template<class ARG_KEY>
>AFX_INLINE UINT AFXAPI HashKey(ARG_KEY key)
>{
> // default identity hash - works for most primitive values
> return (DWORD)(((DWORD_PTR)key)>>4);
>}
>)
>
>plus the following error:
>
>error C2678: binary '==' : no operator found which takes a left-hand
>operand of type 'const SDataType' (or there is no acceptable
>conversion)
> D:\Program Files\Microsoft Visual Studio .NET 2003\Vc7\atlmfc
>\include\afxtempl.h(1544) : see reference to function template
>instantiation 'BOOL CompareElements<KEY,SDataType>(const TYPE *,const
>ARG_TYPE *)' being compiled
> with
> [
> KEY=SDataType,
> TYPE=SDataType,
> ARG_TYPE=SDataType
> ]
>
>refering to
>
>template<class TYPE, class ARG_TYPE>
>BOOL AFXAPI CompareElements(const TYPE* pElement1, const ARG_TYPE*
>pElement2)
>{
> ASSERT(AfxIsValidAddress(pElement1, sizeof(TYPE), FALSE));
> ASSERT(AfxIsValidAddress(pElement2, sizeof(ARG_TYPE), FALSE));
>
> return *pElement1 == *pElement2; // this line
>}
Joseph M. Newcomer [MVP]
email: newcomer(a)flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Arman Sahakyan on

"Vahid" wrote:

> Thanks a lot guys for your suggestions. Since I'm not used to standard
> library (compared to MFC), I picked CMap right away. And before
> reading your messages, I had come up with the following which compiled
> ok:
>
> struct SDataType {
> unsigned long nNamespace;
> unsigned long nDataType;
> SDataType& operator=(SDataType s2) { nNamespace = s2.nNamespace;
> nDataType = s2.nDataType; return *this; }
> bool operator==(SDataType s2) const { return ((nNamespace ==
> s2.nNamespace) && (nDataType == s2.nDataType)); }
> };
>
> template<> AFX_INLINE UINT AFXAPI HashKey(SDataType& key)
> {
> return HashKey((DWORD_PTR) key.nNamespace) + HashKey ((DWORD_PTR)
> key.nDataType);
> }
>
> Arman, how do you compare this with your suggestion. As you see, I do
> not use reference parameters here.
>
> Thanks again,
> Vahid.
>

Vahid,

It's ok to have no const references in this specific case because the fields
of the struct are not so large; my point is they are primitive types and
copying will not be inefficient.

One point about assignment operator. Here you'd rather either to have const
refeneces or to have no assignment operator at all [as noticed David]. The
problem [well not so strictly] is that in the assignment operator you are
invoking the copy-ctor when passing arguments [rhs values] to the operator.
Here will take place a bitwise-copy (shallow copy) of arguments [and then you
copy them again [though member-wise (deep) copy] inside the operator. So the
logic and purpose are harmed. No technical issue.

HashKey issue.
Yes you can do that; it's ok to overload the default behaviour of the MFC
HashKey global function. Another way to go, shown im my first reply, is to
overload the DWORD operator for the struct. Though your way is more specific.
So keep it.



--
======
Arman


From: Vahid on
Thanks again.

I think overriding the hash function was kind of necessary. Because
I'd like to compare to CMap objects and for that I wrote this piece of
code:

bool CComparableDataTypeMap::operator==(CComparableDataTypeMap& map2)
{
POSITION pos = this->GetStartPosition();
SDataType sDataType;
unsigned int nVal, nVal2;
while (pos) {
this->GetNextAssoc(pos, sDataType, nVal);
if (!map2.Lookup(sDataType, nVal2))
return false;
if (nVal != nVal2)
return false;
}

pos = map2.GetStartPosition();
while (pos) {
map2.GetNextAssoc(pos, sDataType, nVal2);
if (!this->Lookup(sDataType, nVal))
return false;
}

return true;
}

It seemed to me that without my hash function this was not working
properly, as I compared to map objects (this: [1,2] -> 1) and (map2:
[1,2] -> 1) and looking up [1,2] in map2 was unsuccessful. It worked
when I added my hash function.

Thanks,
Vahid.

From: Norbert Unterberg on
Vahid schrieb:

> Thanks again.
>
> I think overriding the hash function was kind of necessary. Because
> I'd like to compare to CMap objects and for that I wrote this piece of
> code:
>
> bool CComparableDataTypeMap::operator==(CComparableDataTypeMap& map2)
> {
[...]
> }

Just for your information, the std::map class brings a built-in operator==().
You should definately have a look at the C++ container classes.

Norbert