Newbie question--help with () operator for 2d vector array class

[RobBobSmith]

Hello all,

I'm having trouble with making a 2d vector array class. Would anyone have time to help with a couple of questions?

I think my problem is with overloading (). I notice that this:

template <typename T>
T& Array2d<T>::operator() ( int x, int y)
{
	if( x >= rows || y >= cols ) 
	{
		cout << "Problem with dimensions ";
	 
	 else
		return matrix[x][y];                    
	 }
}

produces the following warning: "Control reaches end of non-void function." I googled this and it appears to mean that the thing the function SHOULD return (matrix[x][y]) is not being returned. This must mean that there is some problem with my indexing. Also, there's a segmentation fault when I try to run my code. Am I correct? If so, can anyone see what the indexing issue is?

If this is not the problem, is there something else I'm missing? I'm pretty new at this and I'm not sure that I understand the STL containers as well as I should. I read that I don't need a destructor, copy constructor or assignment operator. But an example I saw does contain a destructor (I included one just in case). Could someone clarify this? Am I missing a constructor that I do need, or is my destructor a problem?

Also, someone told me that it would be better to use 1-D vectors for making 2-D matrices than the vector "wrapper" style I have used. But I find the vector wrapper style easier to deal with. Is there a particular advantage of using the 1-D style? Is it faster, maybe?

Here is my code:

Header file

#ifndef ARRAY_H
#define ARRAY_H

#include <iostream>
#include <vector>

template <typename T>
class Array2d
{
private:
	unsigned int rows, cols;
	std::vector<std::vector<T> > matrix;          
public:
	// Constructors 
         Array2d( unsigned int rowsPub, unsigned int colsPub );  
	~Array2d(); 
	
	// Functions 
        // Various functions for doing things to the matrix.

	std::vector<T>&       operator()        ( unsigned int x, unsigned int y );
        const std::vector<T>& operator()        ( unsigned int x, unsigned int y ) const;
	
	T& operator()       ( int x, int y );                            
	const T& operator() ( int x, int y ) const;
};
#endif

Definitions file

#include "Array2d.h"
#include <stdlib.h>
#include <iostream>
#include <iomanip>
#include <cmath>
#include <new>

using namespace std;                            

template <typename T>
Array2d<T>::Array2d(unsigned int rowsPub, unsigned int colsPub)
: matrix(rows, vector<T>(cols,0))  
{
    for(int i=0; i<rows; ++i)  
    {
	matrix.push_back(std::vector<T>(cols));
    }	
} 

template <typename T>
Array2d<T>::~Array2d() 
{
}  

// Functions (not included)...

template <typename T>
T& Array2d<T>::operator() ( int x, int y)
{
	if( x >= rows || y >= cols || x < 0 || y < 0) 
	{
		cout << "Problem with dimensions ";
	 
	 else
		 return matrix[x][y];  
        }                  
}   

template <typename T>
const  T& Array2d<T>::operator()( int x, int y) const 
{
	if( x >= rows || y >= cols || x < 0 || y < 0) 
	{
		cout << "Problem with dimensions ";
	
	else
		return matrix[x][y]; 
        }                    
}										

template class Array2d<int>;
template class Array2d<double>;
template class Array2d<long>;

[jonsca]

On line 34, why do you have your else within the braces of your if?

What the compiler is telling you is that there is a path through that method (the path taken if x >= rows || y >= cols || x < 0 || y < 0 is false) where there will be no value returned.

You'll need to rearrange that if/else structure, but also provide a return value to the calling method in the case that the dimensions are not correct.

[RobBobSmith]

Thank you Jonsca.

So this would be more appropriate for the expression within the two operator() braces?

template <typename T>
const  T& Array2d<T>::operator()( int x, int y) const 
{
	if( x >= rows || y >= cols || x < 0 || y < 0) 
	{
		cout << "Problem with dimensions ";
		return matrix[0][0];
	}
	
	else
	{
		return matrix[x][y];                     
	}										
}

[jonsca]

That's the right way to do it with the if/else. Now all paths through that method will return a value.

In doing it the way you did, as long as your calling function could tell the difference between the error condition and the actual value requested as [0][0], but I'm not sure how you could do that without adding an extra parameter. I'm not sure what the best return value would be for the error. That'll be up to your design.

[RobBobSmith]

Thanks a lot!