Good or bad code?

Discussion in 'C++' started by mlt, Mar 8, 2009.

  1. mlt

    mlt Guest

    I am writing a function that takes a reference P to a
    std::vector<std::vector<int> >. Before calling this function I don't know
    the dimensions of P. I need to define those in the function and I need to
    read form all elements in P. Here is what I do:

    void testVector(std::vector<std::vector<int> > & P)
    {

    // Currently hardcoded
    int rows = 3;
    int columns = 3;

    // Initializing vectors of vectors
    P.resize(rows);
    for (int i=0; i<rows; i++)
    {
    P.resize(columns);
    for (int j=0; j<columns; j++)
    {
    P[j] = 33;
    }
    }

    // Reading from vector of vectors.
    int a = 0;
    for (int i=0; i<rows; i++)
    {
    for (int j=0; j<columns; j++)
    {
    P[j] = a;
    a++;
    }
    }


    }

    int main() {

    std::vector<std::vector<int> > P;
    testVector(P);

    int rows = P.size();
    int columns = P[0].size();
    // Printing
    for (int i=0; i<rows; i++)
    {
    for (int j=0; j<columns; j++)
    {
    std::cout << P[j] << std::endl;
    // P[j] = 33;
    }
    }

    return 0;

    }


    But is this how it should be done? Seems a bit clumsy to me.
     
    mlt, Mar 8, 2009
    #1
    1. Advertising

  2. mlt wrote:

    > I am writing a function that takes a reference P to a
    > std::vector<std::vector<int> >. Before calling this function I
    > don't know the dimensions of P. I need to define those in the
    > function and I need to read form all elements in P.


    Must the composite object always be rectangular? If so, you shouldn't
    be handling the nested vector explicitly. Rather, encapsulate it in a
    class that ensures all components are the same length, and lets you
    query the bounds.

    > void testVector(std::vector<std::vector<int> > & P)


    Side remark: That function doesn't test anything, so better call it
    makeTestVector().


    Martin

    --
    Quidquid latine scriptum est, altum videtur.
     
    Martin Eisenberg, Mar 8, 2009
    #2
    1. Advertising

  3. mlt

    Noah Roberts Guest

    mlt wrote:
    > I am writing a function that takes a reference P to a
    > std::vector<std::vector<int> >. Before calling this function I don't
    > know the dimensions of P. I need to define those in the function and I
    > need to read form all elements in P. Here is what I do:
    >
    > void testVector(std::vector<std::vector<int> > & P)
    > {
    >
    > // Currently hardcoded
    > int rows = 3;
    > int columns = 3;
    >
    > // Initializing vectors of vectors
    > P.resize(rows);
    > for (int i=0; i<rows; i++)
    > {
    > P.resize(columns);
    > for (int j=0; j<columns; j++)
    > {
    > P[j] = 33;
    > }
    > }
    >
    > // Reading from vector of vectors.
    > int a = 0;
    > for (int i=0; i<rows; i++)
    > {
    > for (int j=0; j<columns; j++)
    > {
    > P[j] = a;
    > a++;
    > }
    > }
    >
    >
    > }


    The above function has two separate and distinct responsibilities as is
    even pointed out by your comments. This is bad. Split this function
    into two separate functions.
     
    Noah Roberts, Mar 9, 2009
    #3
  4. Noah Roberts wrote:
    > mlt wrote:


    >> void testVector(std::vector<std::vector<int> > & P)
    >> {
    >>
    >> // Currently hardcoded
    >> int rows = 3;
    >> int columns = 3;
    >>
    >> // Initializing vectors of vectors
    >> P.resize(rows);
    >> for (int i=0; i<rows; i++)
    >> {
    >> P.resize(columns);
    >> for (int j=0; j<columns; j++)
    >> {
    >> P[j] = 33;
    >> }
    >> }
    >>
    >> // Reading from vector of vectors.
    >> int a = 0;
    >> for (int i=0; i<rows; i++)
    >> {
    >> for (int j=0; j<columns; j++)
    >> {
    >> P[j] = a;
    >> a++;
    >> }
    >> }
    >>
    >>
    >> }

    >
    > The above function has two separate and distinct
    > responsibilities as is even pointed out by your comments.


    Actually the second comment is just wrong as it's (somewhat
    pointlessly) filling twice.


    Martin

    --
    Quidquid latine scriptum est, altum videtur.
     
    Martin Eisenberg, Mar 10, 2009
    #4
    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. Replies:
    4
    Views:
    377
    Scott Allen
    Dec 2, 2005
  2. Replies:
    0
    Views:
    900
  3. Gene

    Re: Bad code or bad compiler?

    Gene, Jun 1, 2008, in forum: C Programming
    Replies:
    3
    Views:
    287
    Barry Schwarz
    Jun 1, 2008
  4. Willem

    Re: Bad code or bad compiler?

    Willem, Jun 1, 2008, in forum: C Programming
    Replies:
    9
    Views:
    285
    Keith Thompson
    Jun 2, 2008
  5. rantingrick
    Replies:
    44
    Views:
    1,256
    Peter Pearson
    Jul 13, 2010
Loading...

Share This Page