Sign in to follow this  

map/set iterators incompatible

This topic is 3373 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello, I've encountered a problem with a base win32 class I'm developing. I'm using a std:map to map function pointers to a windows message but having thought i had done so correctly, get a run-time assertion error: map/set iterators incompatible Here is the source code to the class responsible...
//+--------------------------------------------------------------------------------+//
//																					//
//	AP_BASEWINDOW																	//
//																					//
//	Last Updated:	16/09/2008														//
//																					//
//	WIN32 wrapper class	used to provide basic functionality of a window				//
//																					//
//+--------------------------------------------------------------------------------+//

#pragma once

#define WIN32_LEAN_AND_MEAN

#include <windows.h>
#include "console.h"
#include <map>
 
using namespace std;

typedef long (*fpMessageHandler)(HWND, long, long);
typedef map<long, fpMessageHandler> MessageMap;
typedef MessageMap::iterator MessageMapIterator;

class AP_BASEWINDOW
{
public:
	AP_BASEWINDOW();
	~AP_BASEWINDOW(void);
	static LRESULT CALLBACK MessageRouter(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam);

	bool Create_Window(HINSTANCE _hInstance, int x_offset, int y_offset, int width, int height, char* title);
	bool changeWindowTitle(LPCTSTR new_title);
	HWND getHandle(){return hWnd;};

	MessageMapIterator GetMessageHandler(long message);
	fpMessageHandler RegisterMessageHandler(long message, fpMessageHandler handler);
	MessageMap getMessageMap(){return message_map;};

protected:	
	HWND hWnd;
	HINSTANCE hInstance;
	virtual LRESULT WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam);	

private:
	MessageMap message_map;

	static long OnWM_CLOSE(HWND hwnd, long wParam, long lParam);
	static long OnWM_DESTROY(HWND hwnd, long wParam, long lParam);
	bool Register_Class(HINSTANCE _hInstance, LPTSTR szWindowClass);
};



#include "AP_BASEWINDOW.h"

AP_BASEWINDOW::AP_BASEWINDOW(void)
{
	hInstance = 0;
}

AP_BASEWINDOW::~AP_BASEWINDOW(void)
{

}

//+--------------------------------------------------------------------------------+//
//																					//	
//	+-MessageRouter(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)				//
//																					//
//																					//
//	Purpose:																		//
//		Determines which instance of the class is the recipient of the generated	//
//		message, then calls the Windows Procedure for that instance.				//
//																					//	
//	Comments:																		//
//		You can not initialize the window class with a class method as the window	//
//		procedure unless it is a static method, so the class also needs a static	//
//		message handler that determines which instance of the class is the			//
//		recipient of the message and calls that instance's window procedure.		//
//																					//
//+--------------------------------------------------------------------------------+//
LRESULT CALLBACK AP_BASEWINDOW::MessageRouter(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
	AP_BASEWINDOW* pWnd = 0;

	//+-TODO:	Possibility that the WM_GETMINMAXINFO message could be sent before

	if(msg == WM_NCCREATE)																			// The WM_NCCREATE message must be handled explicitly 
	{																								// within this message router, and the object pointer stored
		pWnd = reinterpret_cast<AP_BASEWINDOW*>((long)((LPCREATESTRUCT)lParam)->lpCreateParams);	
		::SetWindowLong(hwnd, GWL_USERDATA, (LONG)((LPCREATESTRUCT)lParam)->lpCreateParams);		// Retrieve Window instance from window creation data and associate

		if(pWnd)																					// Make sure access is not trying to be made to a null pointer
		{
			pWnd->hWnd = hwnd;																		// Save the Windows Handle
		}
	}
	else																							// If the window is already created then 32-but value associated with it is already stored
	{
		pWnd = reinterpret_cast<AP_BASEWINDOW*>(::GetWindowLong(hwnd, GWL_USERDATA));				// Retrieve the associated windows instance
	}

	if(pWnd)																						// Make sure access is not trying to be made to a null pointer																					
	{
		MessageMapIterator it;
		it = pWnd->GetMessageHandler(msg);

		if(it != (pWnd->getMessageMap().end()))
		{
			return (it->second)(hwnd, (long)wParam, (long)lParam);
		}

		//return pWnd->WndProc(hwnd, msg, wParam, lParam);											// Call the appropriate windows procedure
	}

	return DefWindowProc(hwnd, msg, wParam, lParam);											// Tell Windows to handle in a default way
}

//+--------------------------------------------------------------------------------+//
//																					//	
//	+-Register_Class(HINSTANCE _hInstance, LPTSTR szWindowClass)					//
//																					//
//																					//
//	Purpose:																		//
//		Will register a window class with a set of default values. The class will	//
//		be registered in the name of the second parameter							//
//																					//	
//	Comments:																		//
//		This method will return true if the windows class has been registered 		//
//		properly, and false if the registration failed								//
//																					//
//+--------------------------------------------------------------------------------+//
bool AP_BASEWINDOW::Register_Class(HINSTANCE _hInstance, LPTSTR szWindowClass)
{
	// Windows Class Structure
	WNDCLASSEX wc;	
	
	// Fill the windows class structure
	wc.cbSize			= sizeof(WNDCLASSEX);
	wc.style			= CS_HREDRAW | CS_VREDRAW;				// Specifies the class styles	
	wc.lpfnWndProc		= (WNDPROC)MessageRouter;				// Pointer to the Windows Procedure	
	wc.cbClsExtra		= 0;									// Specifies the number of extra bytes to allocate following the window-class structure
	wc.cbWndExtra		= 0;									// Specifies the number of extra bytes to allocate following the window instance	
	wc.hInstance		= _hInstance;							// Handle to the instance that contains the window procedure for the class	
	wc.hIcon			= NULL;									// Handle to the class icon
	wc.hCursor			= 0;									// Handle to the class cursor
	wc.hbrBackground	= (HBRUSH)GetStockObject(WHITE_BRUSH);	// Handle to the class background brush
	wc.lpszMenuName		= 0;									// Pointer to a null-terminated character string that specifies the resource name of the 
																// class menu, as the name appears in the resource file
	wc.lpszClassName	= szWindowClass;						// Pointer to a null-terminated string or is an atom. If this parameter is an atom, it must
																// be a class atom created by a previous call to the RegisterClass or RegisterClassEx function
	wc.hIconSm			= LoadIcon(NULL, IDI_APPLICATION);

	if(!RegisterClassEx(&wc))
	{
		// Window class could not be registered
		return false;
	}

	return true;
}

//+--------------------------------------------------------------------------------+//
//																					//
//	+-Create_Window(HINSTANCE _hInstance, int x_offset, int y_offset, int width,	//
//					int height, char* title)										//
//																					//
//	Purpose:																		//
//		This method will setup a basic Win32 window based on the given parameteres	//
//		by first registering a windows class, then creating the window based on		//
//		this class.																	//
//																					//	
//	Comments:																		//
//		The method will return false if the windows class cannot be registered, or	//
//		a window cannot be created based on the registered class.					//
//																					//
//+--------------------------------------------------------------------------------+//
bool AP_BASEWINDOW::Create_Window(HINSTANCE _hInstance, int x_offset, int y_offset, int width, int height, char* title)
{
	hInstance = _hInstance;      // Store instance handle in our global variable

	// Step 1 - Register the Windows Class
	if(!Register_Class(_hInstance, "123"))
	{
		DWORD dw = GetLastError(); 
		MessageBox(NULL, "Window Creation Failed: Window Class could not be registered", "Error", MB_OK );
		return false;
	}
	
	// Step 2 - Create the window	
	hWnd = CreateWindowEx(WS_EX_OVERLAPPEDWINDOW, "123", title, WS_OVERLAPPEDWINDOW, 
								x_offset, y_offset, width, height, NULL, NULL, hInstance, static_cast<AP_BASEWINDOW*>(this));

	if(!hWnd)
	{  
		//1407 Cannot find window class. ERROR_CANNOT_FIND_WND_CLASS

		DWORD dw = GetLastError(); 
		MessageBox(NULL, "Window Creation Failed: CANNOT FIND WND CLASS", "Error", MB_OK );

		return false;
	}

	ShowWindow(hWnd, SW_SHOWNORMAL);
	UpdateWindow(hWnd);

	RegisterMessageHandler(WM_CLOSE, OnWM_CLOSE);
	RegisterMessageHandler(WM_CLOSE, OnWM_DESTROY);

	return true;
}

//+--------------------------------------------------------------------------------+//
//																					//	
//	+-WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)					//
//																					//
//																					//
//	Purpose:																		//
//																					//	
//	Comments:																		//
//																					//
//+--------------------------------------------------------------------------------+//
LRESULT AP_BASEWINDOW::WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
	switch(msg)
	{
		case WM_CLOSE:
		{
			if(hWnd)
			{
				if(!DestroyWindow(hWnd))
				{
					MessageBox(NULL,"AP_BASEGLWINDOW: Could Not Destroy Window","ERROR",MB_OK | MB_ICONINFORMATION);
					hWnd = NULL;										
				}
			}

			//+-TODO:	Insert code to unregister windows class used to create window

			return false;
		}
		case WM_DESTROY:
		{
			PostQuitMessage(0);	
			return false;
		}
		case WM_GETMINMAXINFO:
		{
			LPMINMAXINFO pMMI = (LPMINMAXINFO)lParam;
			pMMI->ptMaxTrackSize.x = 500;
			pMMI->ptMaxTrackSize.y = 500;		
		}
		case WM_KEYDOWN:							// Is A Key Being Held Down?
		{
			if(wParam == VK_RIGHT)
			{
				cout << "Message Handlers: " << message_map.size() << endl;
			}
		}

		default:
			return DefWindowProc(hwnd, msg, wParam, lParam);
	}
}
//+--------------------------------------------------------------------------------+//
//																					//	
//	+-changeWindowTitle(LPCTSTR new_title)											//
//																					//
//																					//
//	Purpose:																		//
//		This method will change the title of this window to the given string		//	
//																					//
//	Comments:																		//
//																					//
//+--------------------------------------------------------------------------------+//
bool AP_BASEWINDOW::changeWindowTitle(LPCTSTR new_title)
{
	return SetWindowText(hWnd, new_title);
}

//+--------------------------------------------------------------------------------+//
//																					//	
//	+-GetMessageHandler(long message)												//
//																					//
//																					//
//	Purpose:																		//
//		This method will return the address of the registered message handler for	//
//		the provided message. If no message handler is regisered then the method	//
//		will return NULL.															//
//																					//
//	Comments:																		//
//																					//
//+--------------------------------------------------------------------------------+//
MessageMapIterator AP_BASEWINDOW::GetMessageHandler(long message)
{
	MessageMapIterator it = message_map.find(message);			// Element is found in O(logn) time given the key (message)

	if(it == message_map.end())									// If there element matching the provided key then the iterator
	{															// will be equal to the last element in the map, so return NULL
		return NULL;
	}

	return it;
}

long AP_BASEWINDOW::OnWM_CLOSE(HWND hwnd, long wParam, long lParam)
{
	DestroyWindow(hwnd);
	return 0;
}

long AP_BASEWINDOW::OnWM_DESTROY(HWND hwnd, long wParam, long lParam)
{
	PostQuitMessage(0);
	return 0;
}

fpMessageHandler AP_BASEWINDOW::RegisterMessageHandler(long message, fpMessageHandler handler)
{
	fpMessageHandler m = NULL;
	MessageMapIterator it = message_map.find(message);					// Element is found in O(logn) time given the key (message)

	if(it != message_map.end())
	{
		m = it->second;
	}

	message_map.insert(pair<long, fpMessageHandler>(message, handler));	// Insert Message Handler into Message Map

	return m;
}


From what I understand, these errors are usually caused by comparisons of 2 iterators, however as far as I can see it's being done correctly. the problem seems to be with both comparing the iterators and calling the function pointer in MessageRouter() Hopefully someone will be able to shed light on the situation for me as I've never encountered an error like this before. Thanks for reading

Share this post


Link to post
Share on other sites
GetMessageHandler() is returning NULL, which means the compiler is trying to create a MessageMapIterator from NULL, which is then compared against another iterator (end()) in the message proc. That's probably where the problem is, you should return the iterator, even if it is equal to end(), then you can test it against end() in the message proc.
EDIT: Actually, you should return either NULL or the function pointer from that function, since there's no reason to return an iterator unless you're doing additional work like removing the handler or iterating the message map.

As an aside, your WM_CLOSE code is a bit redundant; Windows will automatically call DestroyWindow on WM_CLOSE, you should only override it if you need additional behaviour.

Share this post


Link to post
Share on other sites
thanks for replying...

I see what you are saying about returning NULL from GetMessageHandler() so I changed that method to the following, so that it will always return an iterator...


MessageMapIterator AP_BASEWINDOW::GetMessageHandler(long message)
{
MessageMapIterator it = message_map.find(message); // Element is found in O(logn) time given the key (message)

//if(it == message_map.end()) // If there element matching the provided key then the iterator
//{ // will be equal to the last element in the map, so return NULL
//return NULL;
//return message_map.end();
//}

return it;
}



However the same run-time error is appearing even after the change

Share this post


Link to post
Share on other sites
Hmm, interesting. What line does the error occur on? The debugger should trigger a debug break when the assert occurs, and you should be able to look up the call stack to see what line of your code is causing it.

Share this post


Link to post
Share on other sites
Quote:
Original post by ov3r_dr1v3
quite strangely if I step into/over the code during debugging it seems to cause the run-time error when I'm calling CreateWindowEx() during the Create()method
Your window message proc is called during CreateWindowEx (E.g. WM_NCCREATE, WM_CREATE, and some other messages are processed). Try sticking a breakpoint in your message proc too. Or just hit F5 (Assuming visual studio) and let the app crash, then look at the debugger.

Share this post


Link to post
Share on other sites
oh ok, it actually fails then on the iterator comparison in the MessageRouter() method. However, If i remove the comparison and just use the function pointer to call the method regardless of whether its there or not, the same error is encountered.

Share this post


Link to post
Share on other sites
I think I can see what's going wrong.

The iterator returned from the GetMessageHandler function is on the MessageMap that in contained within the AP_BASEWINDOW object however the other side of the comparison is an iterator on a copy of the MessageMap as getMessageMap returns a copy of the map rather than a reference.

Try changing:

MessageMap getMessageMap(){return message_map;};


to:

MessageMap &getMessageMap(){return message_map;};



If this corrects the issue then a better ide may be to change the lines:

typedef MessageMap::iterator MessageMapIterator;

MessageMap getMessageMap(){return message_map;};


to:

typedef MessageMap::const_iterator MessageMapIterator;

const MessageMap &getMessageMap(){return message_map;};



and the GetMessageHandler function to:

MessageMapIterator AP_BASEWINDOW::GetMessageHandler(long message) const
{
return message_map.find(message);
}


Share this post


Link to post
Share on other sites
Thanks that has solved the problem, and the message handler is working as expected now, so thanks for everybody's input.

I do have one question though...whats the advantage of using const_iterators instead of iterators in this case?

Share this post


Link to post
Share on other sites
For const-correctness, probably, because you don't want the user to be able to modify the message map.

However, if you have a method that returns an iterator only so that the user must use it like this:


MessageMapIterator it;
it = pWnd->GetMessageHandler(msg);

if(it != (pWnd->getMessageMap().end()))
{
return (it->second)(hwnd, (long)wParam, (long)lParam);
}


then why not make the GetMessageHandler() method do some more useful work instead:

fpMessageHandler AP_BASEWINDOW::GetMessageHandler(long message)
{
MessageMapIterator it = message_map.find(message);

if(it == message_map.end())
{
return NULL;
}

return it->second;
}


simplifying the usage to

fpMessageHandler handler = pWnd->GetMessageHandler(msg);
if (handler)
{
return handler(hwnd, (long)wParam, (long)lParam);
}


with the additional advantage that the user doesn't need to care that the message handlers are stored in a map (and not some other container).

Share this post


Link to post
Share on other sites
yeah that is a much more logical method lol, thanks for input.

I've changed the code now so from a users point of view they don't have to know about maps, just register and use a message handler

thanks

Share this post


Link to post
Share on other sites
Quote:
Original post by ov3r_dr1v3

I do have one question though...whats the advantage of using const_iterators instead of iterators in this case?


As Visitor said it's for const correctness. It's a good practice to get into always making sure that any pointers, references or methods that aren't meant to to used to change the target data are marked const. It makes sure that you can't accidentally change the data and sometimes it can allow the compiler to make some better optimisations.


Share this post


Link to post
Share on other sites

This topic is 3373 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this