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

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...

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...