Pointers, C++...code not working

Question

This is the question: Write the definition of a function minMax that has five parameters. The first three parameters are integers. The last two are set by the function to the largest and smallest of the values of the first three parameters. The function does not return a value.

The function can be used as follows:

int a = 31, b = 5, c = 19, big, small; 
minMax(a, b, c, &big, &small); /* big is now 31; small is now 5 */ 

This is my code:

void minMax(int x, int y, int z, int* big, int* small)
{
  if (x < y && x < z)

    *small = x;

  else if (y < x && y < z)

    *small = y;

  else if (z < x && z < y)

    *small = z;

  if (x > y && x > z)

    *big = x;

    else if (y > x && y > z)

    *big = y;

  else if (z > x && z > y)

    *big = z;
  }

This is the error I'm getting:

Your function did not change the value of small. Make sure you are dereferencing it in your function.

Not sure what's wrong?

Thanks.

Answer 1

I see one immediate problem.

What do you think will happen when you pass the numbers 1, 1 and 7?

Perhaps you may want to consider the use of <= and >= rather than just < and >.

Since that error message looks nothing like any compiler error I've seen before (and the code is valid syntactically), I'd suggest the message is coming from a test harness which probably:

• sets the big/small values to numbers other than those being passed in (eg, -9999).
• calls the function with test data (eg, 1,1,7).
• checks the output varibales to ensure they've been changed to the correct values.

In addition, it's not the most readable code in the world (no offence intended). If you can structure your code in such a way that its intent is clear from a glance (including comments where appropriate), you'll have hordes of future programmers singing your praises and worshiping your name :-)

Something like this shows the intent a little more clearly (IMNSHO) than lots of those else if constructs:

// Populate big/small based on max/min of x, y and z.

void minMax (int x, int y, int z, int *big, int *small) {
    // Set by default to x, only change if others are bigger.

    *big = x;
     if (y > *big)   *big = y;
     if (z > *big)   *big = z;

    // Same for small but with reversed comparisons.

    *small = x;
     if (y < *small) *small = y;
     if (z < *small) *small = z;
}

Answer 2

The error message

Your function did not change the value of small. Make sure you are dereferencing it in your function.

… appears to come from a test harness provided to you by your teacher.

Anyway, it's correct: there are values that you can choose where your function will not assign anything to *small.

For example, with a, b and c the same value, your function will do nothing at all.

---

Anyway,

for future questions, please provide a complete example program that demonstrates the issue.

So that people will not have to guess and use unreliable telepathy.

---

Also, the assignment calls for you to implement a function with an ungood signature.

It teaches a Bad Way™ to design functions.

Here is a possible ordinary C++ function signature:

void getMinAndMax( int& smallest, int& largest, int a, int b, int c )

Here is an even better signature with modern C++ technology:

std::pair<int, int> minAndMax( int a, int b, int c )

The absence of a get prefix for the latter function's name is because it is an expression-oriented function, like sin and cos (you wouldn't write getSin or getCos, would you?), while the presence of that prefix for the first function is merely to make the name imperative, to reflect that it's not an expression-oriented function but instead an action-oriended function.

Of course, with C++11 one would let the function accept any number of arguments. Except that as I'm writing this, Visual C++ does not yet support that properly. For example, here is the signature of std::min from the C++11 standard library:

template<class T, class Compare>
T min(initializer_list<T> t, Compare comp);

With C++03 one can do that to some degree by accepting a single container argument, of templated type.

Answer 3

I'm not sure what isn't working. It seems like that would basically work but could be better structured.

Maybe something like this:

void minMax(int x, int y, int z, int* big, int* small)
{
    *big = *small = x;

    if (y > *big)
        *big = y;
    if (y < *small)
        *small = y;
    if (z > *big)
        *big = z;
    if (z < *small)
        *small = z;
  }