3

I'm brand new to C++ and am having trouble trying to get a function (which takes an array) to return an array. The function is a very basic sorting algorithm for an array of integers of size 4. What i have is below:

int[] sortArrayAscending(int arrayToSort[3]) {
    int sortedArray[3];
    sortedArray[0] = minOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[1] = lowerMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[2] = higherMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[3] = maxOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    return sortedArray;
}

I think i'm getting really confused with the syntax i need to use (the function calls to min, lower, higher, max all work fine.

I would really appreciate some help.

Thank you

EDIT2: Thank you for all the comments. I have now solved it thanks to @Rook's and @Bob Yoplait's answers. The code is used is:

   int* sortArrayAscending(int arrayToSort[4], int sortedArray[4]) {
    sortedArray[0] = minOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[1] = lowerMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[2] = higherMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[3] = maxOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    return sortedArray;
}

int _tmain(int argc, _TCHAR* argv[])
{
    int testNumbers[4] = {8,14,1,27};
    int testSorted[4];
    sortArrayAscending(testNumbers,testSorted);

    for (int i = 0; i < 4; i++) {
        cout << testSorted[i] << endl;
    }

    system("pause");
    return 0;
}

Thank you for all your help - now time to lookup vectors!

PS I appreciate @Luchian Grigore's solution is most likely the best practise way of doing things, but that wasn't specifically my question

LihO
  • 37,789
  • 9
  • 89
  • 156
rwb
  • 3,892
  • 7
  • 32
  • 56
  • 9
    Use a `std::vector` instead. – hmjd Jun 14 '12 at 08:57
  • 1
    if you have C++11, use `std::array`. – juanchopanza Jun 14 '12 at 08:59
  • 2
    All the array or vector sizes should be 4, not 3. – interjay Jun 14 '12 at 08:59
  • *"The problem [..] asks for arrays"?* Is this an assignment of some sort? – Default Jun 14 '12 at 09:10
  • Do note that when you start using `new` you also have to use `delete`. By returning an `int*` make sure that someone deletes it! By using vectors you don't get this issue, because a copy is made to return the result (if I remember my C++ correctly..) – Default Jun 14 '12 at 09:12
  • 1
    Your main now creates and leaks the array 4 times. Calculate it outside the loop and delete it after the loop. – Michael Anderson Jun 14 '12 at 09:15
  • You are currently calling the `sortArrayAscending` 4 times and you are not deleting any of the returned pointers that you have created. Please read up on the operator `new` before you continue.. – Default Jun 14 '12 at 09:15
  • You can read a bit about it [here](http://stackoverflow.com/questions/6500313/in-c-why-should-new-be-used-as-little-as-possible) – Default Jun 14 '12 at 09:18

7 Answers7

6

Me, I'd probably use std::array<int, 4> if I was using a modern C++ compiler. Deals nicely with bounds checking and memory management and returning from/passing into functions. You can also use existing STL sort mechanisms and functions upon it; no need to reinvent the wheel!

Now, in your case,

int sortedArray[3]; 

is a local variable and you should never return a reference to it directly. You could do something like :

int* sortedArray = new int[4];
// do stuff
return sortedArray;

(also note the size of the array, 4, not 3 in your case!) but in this case you have to remember to delete the array at some point in the future or your application will leak memory.

You can also pass in the array by reference, using an approach like

void sort_array(std::array<int, 4>& the_array);

or

void sort_array(int** the_array)

and in these cases you can modify the array in place, or copy the answer into the argument when you're done sorting.

Rook
  • 5,125
  • 3
  • 31
  • 37
  • Thank you for the response - but c++ complains about "return sortedArray;" when i tried using "int* sortedArray = new int[4];" what is the correct return structure? – rwb Jun 14 '12 at 09:05
4

EDIT: After your edit, you, your function returns a pointer to an array. Should work.

You can either return a pointer or a std::vector.

Note that in your code, you'd be running into undefined behavior, because sortedArray goes out of scope at the end of the method, and the memory is freed.

I'd do this instead:

std::vector<int> sortArrayAscending(int arrayToSort[4]) {
    std::vector<int> sortedArray(4);
    sortedArray.push_back( minOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]));
    sortedArray.push_back(  lowerMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]));
    sortedArray.push_back( higherMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]));
    sortedArray.push_back( maxOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]));
    return sortedArray;
}

Actually, I wouldn't. I'd just use std::sort instead of creating my own function, but that's just me.

Luchian Grigore
  • 236,802
  • 53
  • 428
  • 594
2

You are returning pointer to local variable, which leads to undefined behavior. sortedArray is statically allocated array with automatic storage duration, which means that memory where it resides is being freed once you leave the scope of the function.

You should allocate it dynamically by using new[] or even better: use std::vector instead. If you choose to allocate it by using new[], don't forget to free it by calling delete[] when you don't need this memory anymore.

Also note that int sortedArray[3]; declares an array of size of 3 elements indexed from 0 to 2. If you access 4th element of the array whose size is 3 (if you access the memory "past the last element of the array object"), the behavior is undefined as well.

LihO
  • 37,789
  • 9
  • 89
  • 156
2

As this is C++, suggest using a std::vector<int> instead:

std::vector<int> sortArrayAscending(int arrayToSort[3]) {    
    std::vector<int> sortedArray(4); // Note 4, not 3.
    sortedArray[0] = ...;
    sortedArray[1] = ...;
    sortedArray[2] = ...;
    sortedArray[3] = ...;

    return sortedArray;
}

Note there are several algorithms already available that will perform some of the tasks that you appear to be performing:

hmjd
  • 113,589
  • 17
  • 194
  • 245
1

Use Boost::Array (or std::array in C+11) that provides proper value semantic to C array.

boost::array<int,4> sortArrayAscending(boost::array<int,4>7 arrayToSort) 
{
    boost::array<int,4> sortedArray;
    sortedArray[0] = minOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[1] = lowerMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[2] = higherMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[3] = maxOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    return sortedArray;
}
Joel Falcou
  • 5,837
  • 1
  • 15
  • 34
1

It is not like in Java

Either you pass sortedArray as a parameter to the func

int* sortArrayAscending(int* arrayToSort, int* sortedArray) {
    sortedArray[0] = minOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[1] = lowerMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[2] = higherMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[3] = maxOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    return sortedArray;
}

void toto() {
  int array[4]; // and fill values...
  int sortedArray[4];
  sortArrayAscending(array, sortedArray);
}

or

int* sortArrayAscending(int* arrayToSort) {
    int* sortedArray = new int[4];
    sortedArray[0] = minOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[1] = lowerMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[2] = higherMidOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    sortedArray[3] = maxOfFour(arrayToSort[0],arrayToSort[1],arrayToSort[2],arrayToSort[3]);
    return sortedArray;
}

and then you need to delete the returned array in the second case.

Bob Yoplait
  • 2,322
  • 1
  • 22
  • 34
0

Arrays are always passed by reference to any function in C++. So, just pass your array to the function. Your original array would get sorted and you can then use it in your program. I believe there is no need to return the array explicitly.

Swayam
  • 15,855
  • 13
  • 58
  • 102