Skip to main content
gamedev.net
🔒 Locked

Testing std::map iterator for NULL

Started by _Sigma 7 replies 3.2k views
Original Post
_Sigma
_Sigma
My foray into creating a window wrapper has been pain, if you look at my other posts. I have been able to narrow my problem down to the std::map. So I created an empty project and have been playing with the std::map template, as I have never used it before. First off, the code:

#include <stdio.h>
#include <iostream>
#include <map>
#include <conio.h>

#define MSG_CLOSE 5


typedef unsigned int UINT;
typedef long (* msgHandler)(UINT);
typedef std::map<long,msgHandler>::iterator iter;


std::map<long,msgHandler> messages;

long OnClose(UINT msg)
{
	std::cout << "OnClose called" << std::endl;

	return 1;
}

void AddHandler(UINT msg, msgHandler handler)
{
	messages.insert(std::pair<long,msgHandler>(msg, handler));
}

int main(int argc, char *argv[]) 
{
    

	std::cout << "Adding on close handler" << std::endl;
	AddHandler(MSG_CLOSE,&OnClose);

	std::cout << "Calling on close" << std::endl;

	iter it = messages.find(MSG_CLOSE);

	if (it == NULL)
		std::cout << "OnClose not called" << std::endl;
	else
		(it->second)(5);

	system("PAUSE");

	return 0;
}



The program crashes on the it == NULL line: Obviously this doesn't work as it isn't a pointer. So how should I test it for having not found the msg? On a side note, is there a reason that Visual Assist doesn't parse the file? As it->second underlines the 'second' as if VA didn't know about it. and there is no auto complete for any of the map members.
_Sigma
_Sigma
Alright, must have been blind not to find that on my google foray!

Alright, so why doesn't this work then?
if(pWnd) 	{ 		tyMessageIterator it; 		it = pWnd->GetMessageHandler(message); 		if(it != pWnd->m_MsgHandlers.end())  //here		{			int x =6; 			return (it->second)((*pWnd), hwnd, wparam, lparam);		} 	}

the line 'here' crashes when (or least appears to be the case) it.find returns map::end.

stepping through the code, messages is 1 on first break. step. i hear the windows error ding, but no box. step. message = 31. step. error box
Tempest.exe has triggered a breakpoint

breaks in dbgrptt.c

any idea why this fails ?

Cheers
_goat
_goat
Let's see the GetMessageHandler code, I guess. Apart from not initialising the variable on the same line, I can't see anything wrong.
_Sigma
_Sigma
Quote:
Original post by _goat
Let's see the GetMessageHandler code, I guess. Apart from not initialising the variable on the same line, I can't see anything wrong.

tyMessageIterator TWindow::GetMessageHandler(UINT message){	// m_MsgHandlers is a tyMessageMap instance	tyMessageIterator it = m_MsgHandlers.find(message);	if(it == m_MsgHandlers.end())		return NULL;	return it;}

Since I was using the code from a tut on GD.net, it didn't really cross my mind to look hard here, but I guess
if(it == m_MsgHandlers.end())		return NULL;

might be an issue eh?

would returning m_MsgHandlers.end() work? Or is this bad?
//edit: well she works! any reason not to use this method?
Cheers
Bregma
Bregma
Quote:
Original post by _Sigma
if(it == m_MsgHandlers.end())		return NULL;

might be an issue eh?

would returning m_MsgHandlers.end() work? Or is this bad?


A quality implementation of the map::iterator class would not even compile: it should not have any kind of constructor that would convert from NULL. That, however, is a QOI issue.

Returning end() would be the Right Thing to Do. It is idiomatically correct.

--smw
Stephen M. Webb
Professional Free Software Developer
ApochPiQ
ApochPiQ
Erm, why return m_MsgHandlers.end() there at all? Why not just do this:

tyMessageIterator TWindow::GetMessageHandler(UINT message){	return m_MsgHandlers.find(message);}



Exact same results, less duplicated code.
_Sigma
_Sigma
Quote:
Original post by Bregma
A quality implementation of the map::iterator class would not even compile:

I'm using VS 2005...

Anyways, I'll continue to use this method.

Quote:
Original post by ApochPiQ
Erm, why return m_MsgHandlers.end() there at all? Why not just do this:

tyMessageIterator TWindow::GetMessageHandler(UINT message){	return m_MsgHandlers.find(message);}



Exact same results, less duplicated code.


Because its been a long day :) Will fix that.

Thanks everyone for your help! much appreciated.
Zahlman
Zahlman
An iterator behaves like a pointer, but is not a pointer type.

In particular, a map iterator behaves like a pointer to a std::pair which contains the key (in .first) and value (in .second) of the corresponding map entry.

It *is* somewhat common to translate a map iterator into a pointer-to-value (or to "a value or NULL", if the value type is itself a pointer type):

msgHandler* TWindow::GetMessageHandler(UINT message) {	tyMessageIterator it = m_MsgHandlers.find(message);	return (it == m_MsgHandlers.end()) ? 0 : &(it->second);}


After all, when you call a function like this, you normally don't need to be able to get at the key again ;) OTOH, the returned type is less powerful than a map iterator (that can be a good or a bad thing) and it's a raw pointer (which requires extra care in general).

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.