0

I would like to preface this with I am a junior but relatively experienced developer, but with very little C++ experience.

I have this test method that is supposed to pass an array of characters to find the "O".

TEST(MyTest, ReturnIndexOfO)
{
    Widget unitUnderTest;
    char x = 'X';
    char o = 'O';
    char *row[4] = {&o, &x, &x, &x};
    char *row2[4] = {&x, &o, &x, &x};

    EXPECT_EQ(unitUnderTest.findEmptySpace(*row, 4), 0);
    EXPECT_EQ(unitUnderTest.findEmptySpace(*row2,4 ), 1);
}

And this has properly invoked my findEmptySpace method, which is:

#define WHITE_SPACE 'O'
int Widget::findEmptySpace(char row[], int size)
{
    cout << "The row under test is:\n";
    for(int i = 0; i < size; i++) {
        cout << row[i];
    }
    cout << "\n";
    for(int i = 0; i < size; i++) {
        if(row[i] == WHITE_SPACE) {
            return i;
        }
    }
    return -1;
}

But unfortunately, my output seems indicate that not all of the characters are being read by my findEmptySpace method:

The row under test is:

OX

The row under test is:

X

So even with the truncated data, my first test case passes, but the second test case fails. Any idea why I am not seeing the data correctly?

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
C. Cannon
  • 39
  • 1
  • 6

3 Answers3

3

This expression

Test.findEmptySpace(*row, 4)

is equivalent to the expression

Test.findEmptySpace(row[0], 4)

and the element row[0] is a pointer with the value &o that points to the scalar object o of the type char

char o = 'O';.

So the function call (the inner loops within the function) with such parameters does not make a sense

It seems what you mean is the following

EXPECT_EQ(unitUnderTest.findEmptySpace(row, 4), 0);

and

#define WHITE_SPACE 'O'
int Widget::findEmptySpace(char * row[], int size)
{
    cout << "The row under test is:\n";
    for(int i = 0; i < size; i++) {
        cout << *row[i];
    }
    cout << "\n";
    for(int i = 0; i < size; i++) {
        if( *row[i] == WHITE_SPACE) {
            return i;
        }
    }
    return -1;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1
   char *row[4] = {&o, &x, &x, &x};

This is not an "array of characters". This is an array of four pointers to characters.

unitUnderTest.findEmptySpace(*row, 4)

This passes the first one of these pointers, to findEmptySpace() in its first parameter. The first pointer is a pointer to o.

The second parameter to findEmptySpace() is 4. That's what the above statement does in C++.

The function findEmptySpace() does the following:

    for(int i = 0; i < size; i++) {
        cout << row[i];
    }

So it ends up printing the first four characters from the pointer that was passed to it.

The problem is that the passed in pointer is a pointer to just one character:

&o

That's the first pointer. And this function ends up trying to read the first four character from a pointer that's pointing to only one character.

This results in undefined behavior.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
1

When you call

unitUnderTest.findEmptySpace(*row, 4)

the expression row will decay to a pointer to the first element of row, i.e. to &row[0]. Therefore, the expression *row is equivalent to row[0], which is a pointer to the variable o, i.e. a pointer to a single character.

In the function Widget::findEmptySpace, in the following loop, you use this pointer to a single character as if it were a pointer to 4 characters:

for(int i = 0; i < size; i++) {
    cout << row[i];
}

Since the function argument row (in contrast to the original array with that name) is a pointer to a single character, the only valid index for row would be row[0]. However, you are using the indexes row[0] to row[3], so you are accessing the object out of bounds, causing undefined behavior.

The function signature

int Widget::findEmptySpace(char row[], int size)

is probably not what you want. If you want to pass a pointer to the entire array row from the functon TEST to the function findEmptySpace, then you should pass a pointer to the first element of the array as well as its length. You are already doing the latter properly, but you are not doing the former properly. Since every element of the array is of type char *, a pointer to the first element of the array would be of type char **, i.e. a pointer to a pointer. Therefore, you should change the function signature to the following:

int Widget::findEmptySpace( char **row, int size )

Or, if you prefer, you can use this equivalent, to make it clear that the higher level pointer points to an array:

int Widget::findEmptySpace( char *row[], int size )

Of course, you will have to rewrite your function findEmptySpace to adapt to the different parameter type, so that it dereferences the pointer:

int Widget::findEmptySpace( char *row[], int size )
{
    cout << "The row under test is:\n";

    for( int i = 0; i < size; i++ ) {
        cout << *row[i];
    }

    cout << "\n";

    for( int i = 0; i < size; i++ ) {
        if( *row[i] == WHITE_SPACE ) {
            return i;
        }
    }
    return -1;
}

Now, in the function TEST, you must change the way you are calling the function findEmptySpace, due to the changed function parameter. You should change the lines

EXPECT_EQ(unitUnderTest.findEmptySpace(*row, 4), 0);
EXPECT_EQ(unitUnderTest.findEmptySpace(*row2,4 ), 1);

to:

EXPECT_EQ(unitUnderTest.findEmptySpace( row, 4), 0);
EXPECT_EQ(unitUnderTest.findEmptySpace( row2, 4 ), 1);

Now you are no longer passing the value of the first array element, but you are instead passing a pointer to the first array element, so that the function findEmptySpace can access the entire array properly.

Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39