views:

673

answers:

9

I'm using a 2D matrix in one of my projects. It's something like it is suggested at C++ FAQ Lite.

The neat thing is that you can use it like this:

int main()
{
  Matrix m(10,10);
  m(5,8) = 106.15;
  std::cout << m(5,8);
  ...
}

Now, I have a graph composed of vertices and each vertex has a public (just for simplicity of the example) pointer to 2D matrix like above. Now I do have a pretty ugly syntax to access it.

(*sampleVertex.some2DTable)(0,0) = 0; //bad
sampleVertex.some2DTable->operator()(0,0) = 0; //even worse...

Probably I'm missing some syntactic sugar here due to my inexperience with operator overloading. Is there a better solution?

A: 

This is the best way without changing your code:

//some2DTable is a pointer to a matrix
(*sampleVertex.some2DTable)(0,0)

You could also instead make some2DTable a reference to a matrix instead of a pointer to a matrix. Then you would have simplified syntax as in your first code sniplet.

//some2DTable is a reference to a matrix instead of a pointer to a matrix
sampleVertex.some2DTable(0,0)

Or you could keep some2DTable a pointer to a reference and simply store a reference variable to it and use that in the context of your code block.

Brian R. Bondy
+4  A: 
  1. Consider using references instead of pointers (provided, it can't be null and you can initialize in the constructor).
  2. Consider making a getter or an instance of a matrix wrapper class for a vertex that returns a reference to 2D matrix (provided, it can't be null).

    sampleVertex.some2DTable()(0,0) = 0;
    sampleVertex.some2DTableWrap(0,0) = 0;
    

However, to me it sounds like a non-issue to justify going through all the trouble.

Alex B
+2  A: 

You're basically limited to (*sampleVertex.some2DTable)(0,0). Of course, if you don't need reseating, why not store the actual values in the matrix instead?

Alternatively, make the pointer private and make an accessor (note: the following examples assume a matrix of EntryTypes):

Matrix& Vertex::GetTableRef() 
{
    return *some2DTable; 
}
// or
Matrix::EntryType& Vertex::GetTableEntry(int row, int col)
{
    return (*some2DTable)(row,col);
}

// way later...
myVertex.GetTableRef()(0,0) = 0;
// or...
myVertex.GetTableEntry(0,0) = 0;

Or, just define an inline function to do this for you if you can't change the class Vertex:

// in some header file
inline Matrix& GetTableRef(Vertex& v)
{
    return *v.some2DTable;
}

// or you could do this
inline Matrix::EntryType& GetTableEntry(Vertex& v, int row, int col)
{
    return (*v.some2DTable)(row, col);
}


// later...
GetTableRef(myVertex)(0, 0) = 0;
// or
GetTableEntry(myVertex, 0, 0) = 0;

Finally, don't forget that you don't have to use operator overloading. STL collections implement an at() member function, which is checked, as opposed to operator[] which is unchecked. If you don't mind the overhead of bounds checking, or if you just want to be nonstandard, you could implement at() and then just call myVertex.some2DTable->at(0,0), saving a bit of a syntactic headache altogether.

rlbond
+4  A: 

If you have a pointer to a Matrix, e.g. as a function parameter that you can't make a reference (legacy code, e.g.), you can still make a reference to it (pseudo code):

struct Matrix {
        void operator () (int u, int v) {
        }
};
int main () {
        Matrix *m;
        Matrix &r = *m;
        r (1,1);
}
phresnel
+1  A: 

There is no C++ syntactic sugar that will ease the pain of what you describe:

(*sampleVertex.some2DTable)(0,0) = 0; //bad
sampleVertex.some2DTable->operator()(0,0) = 0; //even worse...

In this situation, I would either have the graph return a reference instead of a pointer, or have the matrix define a function which calls the operator():

inline matrixType &Matrix::get( int x, int y ){ return operator()(x,y); }

Then, the syntax isn't quite as ugly for the vertex example:

sampleVertex.some2DTable->get(0,0) = 0;
Colin
+1  A: 

I would add a function that returns you a ref like rlbond recommends. For a quick fix or if you don't have control over the source of it, i would go with this:

sampleVertex.some2DTable[0](0,0) = 0; // more readable

That's actually equivalent, because the following holds if a is a pointer to a defined class:

*a == *(a + 0) == a[0]

See this long discussion on comp.lang.c++ about that same problem with good answers.

Johannes Schaub - litb
heh, fun hack! :)
A: 

I don't know if it's worth the trouble, but you could do:

class MatrixAccessor {
private:
  Matrix2D* m_Matrix;
public:
  MatrixAccessor(Matrix2D* matrix) : m_matrix(matrix) { }
  double& operator()(int i, int j) const { return (*m_Matrix)(i,j); }
  Matrix2D* operator->() const { return m_Matrix; }
  void operator=(Matrix2D* matrix) { m_Matrix = matrix; }
};

Provided the original operator() returns a reference (as it is in many matrix classes).

Then you provide that MatrixAccessor in your vertex class:

class Vertex {
  Matrix2D* myMatrix;

public:
  MatrixAccessor matrix;
  Vertex(Matrix2D *theMatrix) : myMatrix(theMatrix), matrix(theMatrix) { }
};

Then you can write:

Vertex v;
v.matrix(1,0) = 13;
v.matrix->SomeOtherMatrixOperation();

EDIT

I added const keywords (thanks to @phresnel for bringing up the topic) in order to make the solution semantically equivalent to a solution only presenting a public Matrix2D-pointer.

An advantage of this solution is that constness could be transferred to the matrix object by adding two non-const versions of the operator()() and operator->() (i.e. the matrix cannot be modified on const vertices) and changing the const ones to return a const double& and const Matrix2D* respectively.

That would not be possible when using a public pointer to the matrix object.

MartinStettner
I know it isn't strictly relevant to the answer, but you really should delete myMatrix in the Vertex destructor
e.James
PS: +1 for an elegant solution
e.James
@eJames: Thanks, you're right. Instead, I removed allocation of the Matrix from the constructor. After all, if the Vertex is responsible for creation and destruction, it wouldn't make sense to use a pointer...
MartinStettner
I hate to say, but in a const-correct environment, that code is mostly useless.
phresnel
@phresnel: It would need some tweaking, but I think it isn't a big difference compared to the "original" solution using only a pointer: If the Vertex class just has a public pointer to a Matrix2D, constness wouldn't be applied to the Matrix neither. "Mostly useless" is a bit harsh in my opinion...
MartinStettner
A: 

I'd change the way you get hold of "sampleVertex.some2DTable" so it returns a reference.

Either that or create the reference yourself:

Matrix& m = *sampleVertex.some2DTable;
m(1,2) = 3;
Jimmy J
A: 

You could implement Matrix::operator (int,int) by calling a member function and use that one directly when dealing with pointers.

class Matrix
{
public:
  float ElementAt( int i, int j ) const { /*implement me*/ }
  float operator() ( int i, int j ) const { return ElementAt( i, j ); }
  ...
};

void Foo(const Matix* const p)
{
  float value = p->ElementAt( i, j );
  ...
}

void Bar(const Matrix& m)
{
  float value = m(i,j);
}