Map Help

Started by
5 comments, last by Zahlman 16 years, 5 months ago
So I have been working on a map for an A* algorithm project. The code below complies fine, but encounters a runtime error saying, "Expression: vector subscript out of range" in visual studio. Any help would be greatly appriciated.

#include <iostream>;
#include <vector>

// Node type contains crucial information about nodes on map
struct Node{
	int locX;
	int locY;
	int weight;
	bool isPerson;
};

class MakeMap{
private:
	std::vector< std::vector< struct Node > > map;
	int height, width;
public:
	MakeMap(int x, int y);
	void setProperties(int x, int y);
	int getHeight();
	int getWidth();
};

//sets up height, width, and the map
MakeMap::MakeMap(int x, int y){
	setProperties(x, y);
}

// a settor that sets x, y, and intializes some values in the map
void MakeMap::setProperties(int x, int y){
	height = y;
	width = x;
	map[width][height];

	for(int i = 0; i < height; i++){
		for(int c = 0; c < width; c++){
			map[c].isPerson = false;
			map[c].weight = 9;
		}
	}
}

// returns the height
int MakeMap::getHeight(){
	return height;
}

//returns the widh
int MakeMap::getWidth(){
	return width;
}



[Edited by - elektronjunge on October 26, 2007 1:58:08 PM]
Advertisement
Someone might help you if your format your code correctly (get rid of all the & lt ; & gt ; ) and show with a comment which line produces the error!
What are you expecting the line map[width][height]; to do? It doesn't resize the vector, it just attempts to access beyond the bounds of the vectors.

Replace it with something like this:
map.resize(width);for(int index=0; index < width; ++index){  map[index].resize(height);}



However I would recommend using a std::vector<node> as your map and having the size be height*width instead of using a vector of vectors.
Hello and welcome to C++.

1) In C++, 'struct' and 'class' are basically equivalent; it only affects the defaults on public vs. private member access (and inheritance). Thus, writing 'struct Node' when declaring the vector type, while valid, is redundant. The C++ compiler already knows that 'Node' is a struct; it is smarter than C in that regard.

2) Classes define data types. A name like "MakeMap" is the kind of name that a function has, not a data type. Just call it Map.

3) Having a "set properties" function is useless; just do the work directly in the constructor. If you need to "reset" something, you could just create another one - in normal cases, it's a poorly considered attempt at optimization.

4) Vectors have constructors, too. In particular, they have a constructor which accepts an initial length and a "base" element, and initialize the vector with however many copies of that base. So there is no need to fill them with a loop. Further, the line 'map[width][height];', as noted, is an attempt to *access* that element, not to adjust the vector size. Vectors aren't *that* magical.

5) In C++, we initialize data members of a class with the initializer list, where possible. To make this work, in combination with the vector constructors, we'll need to add a constructor for the node as well. Yes, this works in C++ with structs too (see point 1).

6) Vectors remember their length, so there is no reason for us to do so.

7) Don't #include totally unrelated headers. If your main piece of code uses your Map structure and also does I/O, then include both headers there; don't get the iostream header indirectly from the Node/Map header - because those classes aren't themselves concerned with the I/O.

8) Map locations shouldn't normally need to know where they are on the map - you know automatically because of the context (i.e. the code you used to retrieve the node in the first place). Of course, it doesn't help to have such data if you aren't going to initialize it, anyway.

#include <vector>struct Node {	int weight;	bool isPerson;	Node(int weight, bool isPerson) : weight(weight), isPerson(isPerson) {}};class Map {	private:	std::vector<std::vector<Node> > map;	public:	Map(unsigned int x, unsigned int y) : map(x, std::vector<Node>(y, Node(9, false))) {		assert(x != 0 && y != 0);	}	int getHeight() { return map[0].size(); }	int getWidth() { return map.size(); }};


Yes, it's really that easy.
Thanks for the assistance Zahlman and others. The information that you have about vectors and some general c++ stuff is very handy. I know my code looks weird, especially initializing parts of the node and including x and y locations looks weird. There is actually a good reason for some of the weird coding. This class is sort a step 2 of the map. The initial map will contain everything. Then this class will set up a second map with impassable nodes removed, and cleaned up path lines. But for now this will work because I'm kind of just testing things out.

One quetion though, I threw you code into visual studio, and it had no idea what to do with this line:
assert(x != 0 && y != 0);

Upon closer inspection I wasn't sure what it did either. So what does assert do? And is there something special I need to do to get it working with visual studio

[Edited by - elektronjunge on October 27, 2007 1:51:24 AM]
Quote:Original post by elektronjunge
Upon closer inspection I wasn't sure what it did either. So what does assert do? And is there something special I need to do to get it working with visual studio


To get it working with any compiler, make sure you #include <cassert>. I suspect <vector> on Zahlman's implementation includes <cassert> itself so it went unchecked.

Assertions are used to check that what the programmer thinks about their program is actually true and to check pre-conditions and post-conditions. It is a debugging tool. Use it lots!

In the particular example given, it is assumed that creating a 0x0 size map is a programmer error and should never be attempted (though, IMHO, I don't see why it should be in this particular case, other than it might complicate some of the member functions because additional checks are needed). If two zeros are passed to the Map constructor by the caller, the assert() will pick it up and the program will abort, usually with an error indicating the details of the failed assertion.

When the NDEBUG preprocessor macro is defined, assert(blah) evaluates to nothing (assert is a macro). NDEBUG is typically defined when you build your software with optimizations enabled aka a "release build". This means that assertions have no runtime overheads in release builds.
Quote:Original post by the_edd
Quote:Original post by elektronjunge
Upon closer inspection I wasn't sure what it did either. So what does assert do? And is there something special I need to do to get it working with visual studio


To get it working with any compiler, make sure you #include <cassert>. I suspect <vector> on Zahlman's implementation includes <cassert> itself so it went unchecked.


Where "my implementation" == "a fantasy world inside my own head", yes [smile] I assume too much sometimes and am inconsistent about fixing the header includes in sample code :/

Quote:In the particular example given, it is assumed that creating a 0x0 size map is a programmer error and should never be attempted (though, IMHO, I don't see why it should be in this particular case, other than it might complicate some of the member functions because additional checks are needed).


Or 0xN or Nx0, in general. (I think you misread the logic ;) ) I was particularly concerned about the 0xN case because of the getHeight implementation.

But anyway, boost::multi_array would make a better solution. :P

This topic is closed to new replies.

Advertisement