[c++] Segmentation Fault with STL vector of lists. - Tilted Forum Project Discussion Community

strcrssd · 04-25-2004, 04:41 PM

I can't see a problem with this, but obviously my compiler does:

I'm including an external link for niceness:

http://www.asl.cs.panam.edu/~babbitt/CSCI3333a4/

Can anyone give me a hand?

n0nsensical · 04-25-2004, 07:32 PM

You have to resize the vector HashTable::table before you use it, or supply a size to its constructor. I'm not sure what vector::reserve does but my guess is it allocates memory but doesn't actually resize. If you're not going to be resizing the vector more than once you don't need to bother with reserve, just change it to resize.

KnifeMissile · 04-25-2004, 09:19 PM

Next time, strcrssd, please also post the error message. It probably told you what your problem is and, if you couldn't understand it, chances are that one of us could have.

By the way, I just copied your code into my compiler (VC++ 6.0) and it compiled just fine. VC++ 6.0 is not the world's most compliant compiler so... what was your error message, again?

Also, there are many improvements to your code that can be made but I'll just name one (other than not using the std namespace!). You might want to typedef your containers. Not only will it save you typing but it will also better abstract your containers.
Have you ever wondered why all the STL containers look so similar? It's so that if you were ever to change your mind and decide to use another container, chances are that you can do so and not affect your entire code. However, this will not work if you explicitly declare your type at every insance. So, your class might look like this:

Code:

template&#60class Object&#62&#10class HashTable{&#10	public:&#10		typedef List std::list&#60typename Object>;  // newer compilers might need the typename keyword, here...&#10&#10		HashTable(unsigned long tableSize);&#10		void insert(unsigned long key, Object p);&#10&#10		List retrieve(unsigned long key);&#10		bool retrieve(unsigned long key, List* output);  // this is more efficient because you won't copy an entire container...&#10&#10	protected:&#10		unsigned long hash(unsigned long key);&#10&#10	private:&#10		typedef Table std::vector&#60List>;  // no one needs to know what this is so make it private...&#10&#10		// unsigned long tableSize;  you don't really need this because you'll implicitly store the size in the Table container, table.size()&#10		Table table;&#10};

This way, your for() loop will look like this:

Code:

for(int i=0; i<3; i++){&#10	HashTable::List objList;&#10	if( ht.retrieve( static_cast&#60unsigned long&#62(i), &#38objList) ){&#10		for(HashTable::List::iterator i = objList.begin(); i != objList.end(); i++){&#10			// cout &#60&#60 objList.front().getName() &#60&#60 endl;  this is what you used to have&#10			cout &#60&#60 i-&#62getName() &#60&#60 endl;  // this is probably what you wanted...&#10		}&#10	}&#10}

Notice how you can replace typedef List std::list<typename Object> with typedef List std::deque<typename Object> and no one would care? All you did to replace an entire container type throught your entire code was change one line! Hell, you might even go as far as to replace typedef Table std::vector<List> with typedef Table boost::hash<typename Object>, if one were to exist, someday...

This is the essence of maintainability...

n0nsensical · 04-25-2004, 10:37 PM

Quote:

Originally posted by KnifeMissle
Next time, strcrssd, please also post the error message. It probably told you what your problem is and, if you couldn't understand it, chances are that one of us could have.

By the way, I just copied your code into my compiler (VC++ 6.0) and it compiled just fine. VC++ 6.0 is not the world's most compliant compiler so... what was your error message, again?

It was a segmentation fault aka access violation on Windows, so runtime not compiler error.

KnifeMissile · 04-25-2004, 11:43 PM

Ah, yes--that would make sense. I obviously didn't... read the subject line... or the other link...

Anyway, I think n0nsensical has figured the problem out. You need to call resize() to in order for the constructors of the vector objects to be called. reserve() reallocates memory without calling these constructors...

04-25-2004, 04:41 PM	#1 (permalink)
strcrssd Insane Location: Rio Grande Valley, Texas	[c++] Segmentation Fault with STL vector of lists. I can't see a problem with this, but obviously my compiler does: I'm including an external link for niceness: http://www.asl.cs.panam.edu/~babbitt/CSCI3333a4/ Can anyone give me a hand? __________________ "I can't understand why people are frightened of new ideas. I'm frightened of the old ones." -- John Cage (1912 - 1992)

04-25-2004, 09:19 PM	#3 (permalink)
KnifeMissile Location: Waterloo, Ontario	Next time, strcrssd, please also post the error message. It probably told you what your problem is and, if you couldn't understand it, chances are that one of us could have. By the way, I just copied your code into my compiler (VC++ 6.0) and it compiled just fine. VC++ 6.0 is not the world's most compliant compiler so... what was your error message, again? Also, there are many improvements to your code that can be made but I'll just name one (other than not using the std namespace!). You might want to typedef your containers. Not only will it save you typing but it will also better abstract your containers. Have you ever wondered why all the STL containers look so similar? It's so that if you were ever to change your mind and decide to use another container, chances are that you can do so and not affect your entire code. However, this will not work if you explicitly declare your type at every insance. So, your class might look like this: Code: template&#60class Object&#62&#10class HashTable{&#10 public:&#10 typedef List std::list&#60typename Object>; // newer compilers might need the typename keyword, here...&#10&#10 HashTable(unsigned long tableSize);&#10 void insert(unsigned long key, Object p);&#10&#10 List retrieve(unsigned long key);&#10 bool retrieve(unsigned long key, List* output); // this is more efficient because you won't copy an entire container...&#10&#10 protected:&#10 unsigned long hash(unsigned long key);&#10&#10 private:&#10 typedef Table std::vector&#60List>; // no one needs to know what this is so make it private...&#10&#10 // unsigned long tableSize; you don't really need this because you'll implicitly store the size in the Table container, table.size()&#10 Table table;&#10}; This way, your for() loop will look like this: Code: for(int i=0; i<3; i++){&#10 HashTable::List objList;&#10 if( ht.retrieve( static_cast&#60unsigned long&#62(i), &#38objList) ){&#10 for(HashTable::List::iterator i = objList.begin(); i != objList.end(); i++){&#10 // cout &#60&#60 objList.front().getName() &#60&#60 endl; this is what you used to have&#10 cout &#60&#60 i-&#62getName() &#60&#60 endl; // this is probably what you wanted...&#10 }&#10 }&#10} Notice how you can replace typedef List std::list<typename Object> with typedef List std::deque<typename Object> and no one would care? All you did to replace an entire container type throught your entire code was change one line! Hell, you might even go as far as to replace typedef Table std::vector<List> with typedef Table boost::hash<typename Object>, if one were to exist, someday... This is the essence of maintainability...

04-25-2004, 07:32 PM	#2 (permalink)
n0nsensical Junkie Location: San Francisco	You have to resize the vector HashTable::table before you use it, or supply a size to its constructor. I'm not sure what vector::reserve does but my guess is it allocates memory but doesn't actually resize. If you're not going to be resizing the vector more than once you don't need to bother with reserve, just change it to resize.

04-25-2004, 11:43 PM	#5 (permalink)
KnifeMissile Location: Waterloo, Ontario	Ah, yes--that would make sense. I obviously didn't... read the subject line... or the other link... Anyway, I think n0nsensical has figured the problem out. You need to call resize() to in order for the constructors of the vector objects to be called. reserve() reallocates memory without calling these constructors...