Request for code review

[P Kenter]

Dear all.

I'm currently writing unit tests in C++ and am checking 2D or 3D
'arrays' for their dimension. These 'arrays' are implemented as
vectors of vectors. The problem is, the vectors of a vector can differ
in size if they are initialized incorrectly.

I've written a routine to check for this and would like your view on
style, etc. If I've implemented something that is part of the
language, please let me know.I couldn't find it.

This is the first time I've written a routine that uses templates so
I'd especially appreciate comments on that aspect.

The code is appended below. I hope the commments make it clear how it
works.

The 2nd and 3rd parameter together make up the dimensions that the
first parameter must have. I would like to combine parameter 2 and 3
into one parameter that is a vector of integers. However I would also
like to have brief init statements like this:

uint aDim_2_3_4[] = {2, 3, 4};

Any tips on that?

Regards, Pepijn Kenter



#include <assert.h>
#include <vector>
#include <iostream>

using namespace std;

// Returns true if vVector is a multi dimensional array of size
// (aDim[0] x aDim[1] x ...)
// param vVector the Vector to be checked
// param nDims the number of dimensions vVector should have
// param aDim array containing the dimensions vVector should have.
// aDim should be array of size(nDims), this is not checked.

template<class T> bool CorrectDimensions(const vector<T>& vVector,
const uint nDims,
const uint aDim[])
{
if (nDims == 0) {
return false;
} else {
// check if current dimension is correct
if (vVector.size() == aDim[0]) {

// check the next dimension for incorrect values
// if this is the last dimension (i.e. nDims-1==0) vVector
// must have no trailing dimensions. In that case the
// recursion shall return true because the non-vector
// routine is called.

for (uint nIdx = 0; nIdx < vVector.size(); nIdx++) {
if (!CorrectDimensions(vVector[nIdx], nDims-1,
aDim+1)) {
return false;
}
}
return true;
} else {
return false; // current dimension not correct size
}
}
assert(false); // should not come here
}

// Every type that is not a vector must have 0 dimensions.
template<class T> bool CorrectDimensions(const T& vVector,
const uint nDims,
const uint aDim[])
{
if (nDims == 0) {
return true;
} else {
return false;
}
}


int main(void)
{

uint aDim_2[] = {2};
uint aDim_2_3[] = {2, 3};
uint aDim_2_3_4[] = {2, 3, 4};

vector< vector< double > > vVector;
vVector.resize(2);
vVector[0].resize(3); // vVector[0] and vVector[1] differ in size!
vVector[1].resize(2);

cout << "Initialising vVector" << endl;
cout << "Check 2x3: " <<
CorrectDimensions(vVector, 2, aDim_2_3) << endl;
cout << "Check 2: " <<
CorrectDimensions(vVector, 1, aDim_2) << endl;
cout << "Check 2x3x4: " <<
CorrectDimensions(vVector, 3, aDim_2_3_4) << endl;
cout << endl;

// make vVector now a 2x3 array.

cout << "Resizing vVector" << endl;
vVector[1].resize(3);
cout << "Check 2x3: " <<
CorrectDimensions(vVector, 2, aDim_2_3) << endl;
cout << "Check 2: " <<
CorrectDimensions(vVector, 1, aDim_2) << endl;
cout << "Check 2x3x4: " <<
CorrectDimensions(vVector, 3, aDim_2_3_4) << endl;
cout << endl;

return 0;
}

 

[Victor Bazarov]

I'm currently writing unit tests in C++ and am checking 2D or 3D
'arrays' for their dimension. These 'arrays' are implemented as
vectors of vectors. The problem is, the vectors of a vector can differ
in size if they are initialized incorrectly.

"Incorrectly"? What if you _want_ them to differ in size?

I've written a routine to check for this and would like your view on
style, etc. If I've implemented something that is part of the
language, please let me know.I couldn't find it.

This is the first time I've written a routine that uses templates so
I'd especially appreciate comments on that aspect.

The code is appended below. I hope the commments make it clear how it
works.

The 2nd and 3rd parameter together make up the dimensions that the
first parameter must have. I would like to combine parameter 2 and 3
into one parameter that is a vector of integers. However I would also
like to have brief init statements like this:

uint aDim_2_3_4[] = {2, 3, 4};

Any tips on that?

Regards, Pepijn Kenter



#include <assert.h>

You're a grown man, use

#include <cassert>

..

#include <vector>
#include <iostream>

using namespace std;

// Returns true if vVector is a multi dimensional array of size
// (aDim[0] x aDim[1] x ...)
// param vVector the Vector to be checked
// param nDims the number of dimensions vVector should have
// param aDim array containing the dimensions vVector should have.
// aDim should be array of size(nDims), this is not checked.

template<class T> bool CorrectDimensions(const vector<T>& vVector,
const uint nDims,
const uint aDim[])
{
if (nDims == 0) {
return false;
} else {
// check if current dimension is correct
if (vVector.size() == aDim[0]) {

// check the next dimension for incorrect values
// if this is the last dimension (i.e. nDims-1==0) vVector
// must have no trailing dimensions. In that case the
// recursion shall return true because the non-vector
// routine is called.

for (uint nIdx = 0; nIdx < vVector.size(); nIdx++) {
if (!CorrectDimensions(vVector[nIdx], nDims-1,
aDim+1)) {

Shouldn't it be

if (!CorrectDimensions(vVector[nIdx], nDims-1, aDim+nIdx)) {

???

I am not sure I understand the algorithm, probably. What if your
original vector is a vector<vector<vector<T> > > ? What the 'aDim'
array would look like then?

Imagine that you have two vectors, the first is three vectors, and
so on:

(2 (3 (3 (5 6 7)) (2 (8 9)) (2 (2 2))) (2 (3 (3 4 5)) (2 (5 4))))
3: ^ ^ ^ 2: ^ ^ 2: ^ ^ 3: ^ ^ ^ 2: ^ ^
3: ^^^^^^^^^ ^^^^^^^ ^^^^^^^ 2: ^^^^^^^^^ ^^^^^^^
2: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^

(so, skip the parentheses and you get

{ 2 3 3 5 6 7 2 8 9 2 2 2 2 3 3 4 5 2 5 }

Is that the idea? Isn't it overly complicated to keep?

return false;
}
}
return true;
} else {
return false; // current dimension not correct size
}
}
assert(false); // should not come here
}

// Every type that is not a vector must have 0 dimensions.
template<class T> bool CorrectDimensions(const T& vVector,
const uint nDims,
const uint aDim[])
{
if (nDims == 0) {
return true;
} else {
return false;
}
}


int main(void)
{

uint aDim_2[] = {2};
uint aDim_2_3[] = {2, 3};

Shouldn't this be {1, 3} ?

uint aDim_2_3_4[] = {2, 3, 4};

vector< vector< double > > vVector;
vVector.resize(2);
vVector[0].resize(3); // vVector[0] and vVector[1] differ in size!
vVector[1].resize(2);

cout << "Initialising vVector" << endl;
cout << "Check 2x3: " <<
CorrectDimensions(vVector, 2, aDim_2_3) << endl;
cout << "Check 2: " <<
CorrectDimensions(vVector, 1, aDim_2) << endl;
cout << "Check 2x3x4: " <<
CorrectDimensions(vVector, 3, aDim_2_3_4) << endl;
cout << endl;

// make vVector now a 2x3 array.

cout << "Resizing vVector" << endl;
vVector[1].resize(3);
cout << "Check 2x3: " <<
CorrectDimensions(vVector, 2, aDim_2_3) << endl;
cout << "Check 2: " <<
CorrectDimensions(vVector, 1, aDim_2) << endl;
cout << "Check 2x3x4: " <<
CorrectDimensions(vVector, 3, aDim_2_3_4) << endl;
cout << endl;

return 0;
}

OK, I probably thought you were going to be a bit more generic and
program a way to check the dimensions of any combination of vectors.
It might actually work, you know.

Victor

 

[jmoy]

Dear all.

I'm currently writing unit tests in C++ and am checking 2D or 3D
'arrays' for their dimension. These 'arrays' are implemented as
vectors of vectors. The problem is, the vectors of a vector can differ
in size if they are initialized incorrectly.

Why not make the construction of incorrect 'arrays' impossible using
templates:

template <class T, int dim> class Vecublic std::vector<T>{
public:
Vec():std::vector<T>(dim){};
};

template <class T, int dim1,int dim2=dim1> class Array2D
ublic Vec<Vec<T,dim2>,dim1>{
};

and then use them like

main()
{
Array2D<float,5,6> gauss;
Array2D<double,3> square;
gauss[2][3]=2.718;
}

and so on. As written these classes are inefficient if T's default
constructor is expensive since all elements are initialized with their
default values. However, this can be rectified by having more complex
constructors for Vec which pass on their arguments to the std::vector
constructors. As far as I know the standard does not allow default
initializer lists for user defined types, including types defined in
the standard library.

 

[P Kenter]

P Kenter wrote:

Hi, sorry for the late reply, I didn't have acces to usenet and google
had some troubles.

"Incorrectly"? What if you _want_ them to differ in size?

If they differ in size, I wouldn't call them 2D arrays, but arrays of
arrays. Since they are meant to be 2D arrays, it is a bug if they
differ in size.

#include <assert.h>

You're a grown man, use

#include <cassert>

Ok.

.

#include <vector>
#include <iostream>

using namespace std;

// Returns true if vVector is a multi dimensional array of size //
(aDim[0] x aDim[1] x ...)
// param vVector the Vector to be checked
// param nDims the number of dimensions vVector should have
// param aDim array containing the dimensions vVector should have.
// aDim should be array of size(nDims), this is not checked.

template<class T> bool CorrectDimensions(const vector<T>& vVector,
const uint nDims,
const uint aDim[])
{
if (nDims == 0) {
return false;
} else {
// check if current dimension is correct
if (vVector.size() == aDim[0]) {

// check the next dimension for incorrect values
// if this is the last dimension (i.e. nDims-1==0) vVector
// must have no trailing dimensions. In that case the
// recursion shall return true because the non-vector
// routine is called.

for (uint nIdx = 0; nIdx < vVector.size(); nIdx++) {
if (!CorrectDimensions(vVector[nIdx], nDims-1,
aDim+1)) {

Shouldn't it be

if (!CorrectDimensions(vVector[nIdx], nDims-1, aDim+nIdx)) {

???

I am not sure I understand the algorithm, probably. What if your
original vector is a vector<vector<vector<T> > > ? What the 'aDim'
array would look like then?

Imagine that you have two vectors, the first is three vectors, and
so on:

(2 (3 (3 (5 6 7)) (2 (8 9)) (2 (2 2))) (2 (3 (3 4 5)) (2 (5 4))))
3: ^ ^ ^ 2: ^ ^ 2: ^ ^ 3: ^ ^ ^ 2: ^ ^
3: ^^^^^^^^^ ^^^^^^^ ^^^^^^^ 2: ^^^^^^^^^ ^^^^^^^
2: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^

(so, skip the parentheses and you get

{ 2 3 3 5 6 7 2 8 9 2 2 2 2 3 3 4 5 2 5 }

Is that the idea? Isn't it overly complicated to keep?

If I've got a vector<vector<T> > like in my main routine then the
correct aDim must have 2 elements and nDim must therefore be 2. (sorry
for not taking a vector<vector<vector<T> > >, that is too much typing


Like this:

vector< vector< double > > vVector;
vVector.resize(2);
vVector[0].resize(3);
vVector[1].resize(3);

vVector is now a 2-Dimensional (2x3) array so the correct nDim = 2 and
the correct aDim= {2, 3}

All other aDims must return false.

So first it is checked that the first dimension is aDim[0] (i.e. 2)
and then via the recursion is it checked that the second dimension is
aDim[1] (i.e. 3)

[snip]

OK, I probably thought you were going to be a bit more generic and
program a way to check the dimensions of any combination of vectors.
It might actually work, you know.

Victor

I'm pretty sure the algorithm works, the function calls in the main()
routine give the right results.

Pepijn.

 

[P Kenter]

Why not make the construction of incorrect 'arrays' impossible using
templates:

That's a good idea.

Pepijn.