Request for code review

Discussion in 'C++' started by P Kenter, May 28, 2004.

  1. P Kenter

    P Kenter Guest

    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;
    }
     
    P Kenter, May 28, 2004
    #1
    1. Advertising

  2. P Kenter wrote:
    > 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
     
    Victor Bazarov, May 28, 2004
    #2
    1. Advertising

  3. P Kenter

    jmoy Guest

    (P Kenter) wrote in message news:<>...
    > 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 Vec:public std::vector<T>{
    public:
    Vec():std::vector<T>(dim){};
    };

    template <class T, int dim1,int dim2=dim1> class Array2D
    :public 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.
     
    jmoy, May 29, 2004
    #3
  4. P Kenter

    P Kenter Guest

    Victor Bazarov wrote:
    > P Kenter wrote:
    >


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

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


    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, Jun 2, 2004
    #4
  5. P Kenter

    P Kenter Guest

    (jmoy) wrote in message news:<>...
    > (P Kenter) wrote in message news:<>...
    > > 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:
    >


    That's a good idea.

    Pepijn.
     
    P Kenter, Jun 2, 2004
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Code Review Request

    , Jun 30, 2005, in forum: Java
    Replies:
    2
    Views:
    529
    Andrew Thompson
    Jun 30, 2005
  2. Ben Hanson

    Request for Code Review

    Ben Hanson, Jul 2, 2004, in forum: C++
    Replies:
    19
    Views:
    678
    Chris Gordon-Smith
    Jul 4, 2004
  3. Debashish Chakravarty

    request for code review

    Debashish Chakravarty, Nov 18, 2003, in forum: C Programming
    Replies:
    3
    Views:
    397
    Robert Stankowic
    Nov 18, 2003
  4. Adam Monsen

    code review request: basic file I/O

    Adam Monsen, Aug 16, 2004, in forum: C Programming
    Replies:
    9
    Views:
    345
    Arthur J. O'Dwyer
    Aug 19, 2004
  5. www
    Replies:
    51
    Views:
    1,500
Loading...

Share This Page