patterncppModerate
Reversing the elements of an array
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.
-
-
If you have C++11, prefer
Your array initialization, for instance, would look like this:
If you use the above array or another STL container, the loop counters in
Also, as @thelamb mentioned, pre-increment (
-
Prefer iterators to raw pointers in C++:
-
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):
-
I'd avoid the extra newlines in
So, instead of this:
you could have this:
For comparison, here's an idiomatic alternative using
-
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
\ninstead ofstd::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.