HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Reversing the elements of an array

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arraythereversingelements

Problem

Any way to make this more concise/efficient?

// Write a loop that reverses the elements of an array.
#include 
#include 

const int SIZE = 10;

void reverse(int iArrayRef1 []);
void display(int iArrayRef2 []);

int main()
{
    int iArray[SIZE] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

    // Display in the original order.
    std::cout << "Forwards:\n";
    display(iArray);

    // Reverse the array.
    reverse(iArray);

    std::cout << std::endl;

    // Display in reversed order.
    std::cout << "Backwards:\n";
    display(iArray);
    return 0;
}

void reverse(int iArrayRef1 [])
{
    int temp = 0;
    int* indexPtrBeg = iArrayRef1;
    int* indexPtrEnd = iArrayRef1 += (SIZE-1);

    for (int index = 0; index < (SIZE/2); index++)
    {
        temp = *indexPtrEnd;
        *indexPtrEnd = *indexPtrBeg;
        *indexPtrBeg = temp;
        indexPtrEnd--;
        indexPtrBeg++;
    }
}

void display(int iArrayRef2 [])
{
    for (int index = 0; index < SIZE; index++)
    {
        std::cout << std::setw(2) << iArrayRef2[index] << " ";
    }
    std::cout << std::endl;
}

Solution

For an implementation without the STL, this is pretty good. There's no risky memory-management going on with the raw pointers, and your sorting isn't needlessly slow (compared to bubble sort). I'll base my review on the STL anyway (for future reference), along with some other general tips.

-
SIZE should be of type std::size_t. It's preferred to use an unsigned integer type for sizes, plus you'll have a larger range (no negative values are part of it).

-
If you have C++11, prefer std::array (or any other STL container) over C-style arrays. This also makes it needless to pass the array size since STL containers already know their own.

Your array initialization, for instance, would look like this:

std::array iArray = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };


If you use the above array or another STL container, the loop counters in reverse() and display() should be of type std::size_type. This will ensure:

  • the supported size type for this container is used



  • the loop will be able to access any number of elements



Also, as @thelamb mentioned, pre-increment (++index) is preferred because it will avoid extra copies. This is especially important for user-defined types (such as std::array), while post-increment is still okay with native types (such as int).

-
Prefer iterators to raw pointers in C++:

// with C++11

auto begin = std::begin(container);
auto end   = std::end(container);


// without C++11

std::ContainerType::iterator begin = std::begin(container);
std::ContainerType::iterator end   = std::end(container);


-
You could compare your reverse algorithm to the idiomatic STL method with regards to efficiency (if you were to perform a speed test on them):

std::reverse(container.begin(), container.end());


-
I'd avoid the extra newlines in display(). The function's purpose is solely to display the array, plus you'll be forced to have the newline with each call. Two things for this:

  • add the newlines before and/or after the function call instead



  • use \n instead of std::endl (the latter also flushes the buffer, which takes longer)



So, instead of this:

std::cout << std::endl;

std::cout << "Backwards:\n";
display(iArray);


you could have this:

std::cout << "\nBackwards:\n"; // another '\n' at the front
display(iArray);
std::cout << "\n";             // newline here instead of in display()


For comparison, here's an idiomatic alternative using templates (any element type is supported). It uses an std::vector instead of an array, saving you size constraints. It also displays the vector in forward and in reverse using an iterator, instead of mutating it for the reverse.

#include  // std::copy, std::reverse_copy
#include 
#include   // std::ostream_iterator
#include 

template 
void displayForward(std::vector const& vec)
{
    std::copy(vec.cbegin(), vec.cend(),
        std::ostream_iterator(std::cout, " "));
}

template 
void displayReverse(std::vector const& vec)
{
    std::reverse_copy(vec.cbegin(), vec.cend(),
        std::ostream_iterator(std::cout, " "));
}

int main()
{
    std::vector vec = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

    std::cout << "Forward:\n";
    displayForward(vec);

    std::cout << "\nBackwards:\n";
    displayReverse(vec);
}

Code Snippets

std::array<int, SIZE> iArray = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
// with C++11

auto begin = std::begin(container);
auto end   = std::end(container);
// without C++11

std::ContainerType::iterator begin = std::begin(container);
std::ContainerType::iterator end   = std::end(container);
std::reverse(container.begin(), container.end());
std::cout << std::endl;

std::cout << "Backwards:\n";
display(iArray);

Context

StackExchange Code Review Q#32931, answer score: 12

Revisions (0)

No revisions yet.