0

Can somebody please help me get this code working? The Reverse method is to take a char array parameter, and return a reversed array. Then I want to print it out in main

#include <iostream>
#include <iomanip>
using namespace std;

char[] Reverse(char ch[]) {
    char arr[sizeof(ch)];

    for (int i=0; i< sizeof(ch); i++){
        arr[i] = ch[sizeof(ch) -i -1];
    }

    return arr;
}

int main() {
    char ch1[] = {'a', 'b', 'c'};
    char ch2[] = Reverse(ch1);


    cout << ch2[0] << endl;
    return 0;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
Maria
  • 1
  • 1
    `sizeof(ch)` on my platform is 8. It appears you want it to be the size of the array. You'll need to change Reverse to have `char ch[], size_t len` parameters. And `char arr[len];` would be a variable length array, which is not supported in standard C++. You'll need `char* arr = new char[len];` and `delete[] arr` later on. Assuming you cannot use `std::string` or `std::vector`. – Eljay May 27 '20 at 22:27
  • This is C++ so learn about `std::string` *right away* as it can avoid a lot of pitfalls caused by C strings. Secondly, you're returning a pointer to a temporary array which is *undefined behaviour* so this code is invalid. `Reverse()` must either reverse in-place on the existing buffer, take a mutable argument for the destination, or allocate memory for a new buffer. – tadman May 27 '20 at 22:28
  • 2
    @tadman "_This code only works because `sizeof(ch)` is 1_" `sizeof(ch)`, inside of the `Reverse` function is equivalent to `sizeof (char*)`, which is either 4, or 8, on most platforms. – Algirdas Preidžius May 27 '20 at 22:28
  • @AlgirdasPreidžius You're right, it's worse than I thought. It has no place being there. – tadman May 27 '20 at 22:28
  • 1
    Any function that works on raw C strings either needs to use `strlen()` to figure out the length, **NOT** `sizeof()`, or it must take an explicit `size_t len` argument. – tadman May 27 '20 at 22:29
  • 1
    @tadman • this isn't a raw C string, it's a C array of char that is not `'\0'` terminated. Which is also returning local variable that will be destructed. Ugh, so much ow ow ow. – Eljay May 27 '20 at 22:36
  • 1
    @Eljay Good observation, though that is another problem with this code. A lot of this could be cleaned up by using `std::string`, or if there's constraints that prevent that, a more conventional C-style approach. – tadman May 27 '20 at 22:37
  • [Using sizeof on arrays passed as parameters](https://stackoverflow.com/questions/9443957/) – Remy Lebeau May 27 '20 at 22:38

1 Answers1

0

For starters this function declaration

char[] Reverse(char ch[]);

is incorrect. Functions may not have an array return type.

You could declare the function like

char * Reverse(char ch[]);

but as your function does not deal with strings then in this case it is more logical consistent to declare your function like

void Reverse( char ch[], size_t n );

As the function parameter of the array type is adjusted by the compiler to pointer to the element type of the array like

void Reverse( char *ch, size_t n );

then within the function the parameter ch is a pointer. As a result this expression sizeof( ch ) used within the function yields the size of pointer that depending on the used system is equal to either 4 or 8. so it is why I specified the second parameter of the function because you need explicitly to pass the array size if the array is not passed to the function by reference.

Within the function you are trying to return pointer to the first element of a local array

char arr[sizeof(ch)];
//...
return arr;

that makes the returned pointer invalid because the array will not be alive after exiting the function.

As the first parameter of the function is declared without the qualifier const then it means that the function needs to reverse the original array instead of making a reversed copy of the original array.

Pay attention to that this statement in main

cout << ch2[0] << endl;

does not make great sense. It outputs only the first element of the array.

Taking all this into account the function can be defined the following way

#include <iostream>
#include <utility>

void Reverse( char *s, size_t n )
{
    for ( size_t i = 0; i < n / 2; i++ )
    {
        std::swap( s[i], s[n-i-1] );
    }
}

int main() 
{
    char s[] = { 'a', 'b', 'c' };

    std::cout.write( s, sizeof( s ) ) << '\n';

    Reverse( s, sizeof( s ) );

    std::cout.write( s, sizeof( s ) ) << '\n';

    return 0;
}

The program output is

abc
cba

An alternative approach is to write a template function. In this case the second parameter is not required.

Here you are.

#include <iostream>
#include <utility>

template <size_t N>
void Reverse( char ( &s )[N] )
{
    for ( size_t i = 0; i < N / 2; i++ )
    {
        std::swap( s[i], s[N-i-1] );
    }
}
int main() 
{
    char s[] = { 'a', 'b', 'c' };

    std::cout.write(s, sizeof( s ) ) << '\n';

    Reverse( s );

    std::cout.write(s, sizeof( s ) ) << '\n';

    return 0;
}

If you want to write a function that reverses a string stored in a character array then the function declaration can look like

char * Reverse( char s[] );

Here is one more demonstrative program.

#include <iostream>
#include <utility>
#include <cstring>

char * Reverse( char *s )
{
    for ( size_t i = 0, n = std::strlen( s ); i < n / 2; i++ )
    {
        std::swap( s[i], s[n-i-1] );
    }

    return s;
}

int main() 
{
    char s[] = { 'a', 'b', 'c', '\0' };

    std::cout << s << '\n';

    std::cout << Reverse( s ) << '\n';

    return 0;
}

The program output will be the same as it is shown above that is

abc
cba

Pay attention to that there is standard algorithm std::revrese that you could use for your original array

#include <iostream>
#include <iterator>
#include <algorithm>

int main() 
{
    char s[] = { 'a', 'b', 'c' };

    std::cout.write( s, sizeof( s ) ) << '\n';

    std::reverse( std::begin( s ), std::end( s ) );

    std::cout.write( s, sizeof( s ) ) << '\n';

    return 0;
}

If the array contains a string then in general case a call of the algorithm will look the following way

#include <iostream>
#include <algorithm>
#include <cstring>

int main() 
{
    char s[] = { 'a', 'b', 'c', '\0' };

    std::cout << s << '\n';

    std::reverse( s, s + std::strlen( s ) );

    std::cout << s << '\n';

    return 0;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335