1

I'm aware that there are many questions like mine, but reading several didn't help me. Probably because I'm new at programming and having a hard time with pointers.

As an exercise, I'm trying to create a function in c++ to reverse an inputted string. Here is my function:

char* reverse(const char* t)
{
    int j, k;
    char* aString = new char[100];

    for(j=0, k=strlen(t)-1; j < strlen(t); j++, k--)
    {
        aString[j]=t[k];
    }

    aString[j+1]='\0';

    return aString;
}

However, the input doesn't get reversed at all. What am I doing wrong?

Oleksiy
  • 37,477
  • 22
  • 74
  • 122
spartanhooah
  • 183
  • 4
  • 15
  • Please provide a complete, compileable example that demonstrates the problem. The code works okay for me, within certain limitations (i.e. no strings over 99 characters) http://ideone.com/NVWt25 – Benjamin Lindley Aug 16 '13 at 01:42
  • 3
    Your function is not reversing the original string -- it is returning a pointer to a different string. – SheetJS Aug 16 '13 at 01:47
  • 1
    You should store the result of `strlen(t)` in a local variable before the loop, because it takes some time, especially with a large string. You are calling it every loop with the middle for loop statement. – Neil Kirk Aug 16 '13 at 01:49
  • This isn't quite a pointers exercise as much as it is a string handling exercise. As said before, you aren't reversing the _original_ string at all here. – charmlessCoin Aug 16 '13 at 02:03
  • Although the answers are correct, if the point is to learn pointers, some answers from [here](http://stackoverflow.com/questions/198199/how-do-you-reverse-a-string-in-place-in-c-or-c) might be more helpful. – charmlessCoin Aug 16 '13 at 02:14
  • The problem was in my main function - I was outputting the input string, expecting it to be reversed, but failing to take into account the "const". D'oh – spartanhooah Aug 16 '13 at 11:57

2 Answers2

0

In C++, you have a better, cleaner, safer, easier and more readable option - std::string.

Here is a simple example of a function reverse() that returns a reversed std::string:

void swap(string& str, int index1, int index2) {

    char temp = str[index1];
    str[index1] = str[index2];
    str[index2] = temp;

}

string reverse(string str) {

    int size = str.size();

    for (int i = 0; i < size / 2; i++) 
        swap (str, i, size - i - 1);

    return str;

}
Oleksiy
  • 37,477
  • 22
  • 74
  • 122
  • Or just `std::swap(str[i], str[size-i-1])` - no need to define your own swap. – MSalters Aug 16 '13 at 09:05
  • @MSalters Defining your own functions is a good exercise; also, `std::swap` needs an extra `include` - is it worth it, just for 3 trivial lines of code? – Oleksiy Aug 16 '13 at 11:51
  • Sure, but you could also write your own alternative for `operator[]`. The exercise was just to write your own alternative for `std::reverse`, and then it's probably sufficient to write just one level deep, or alternatively implement it without any Standard Library function. (The famous zero, one, infinity rule). – MSalters Aug 16 '13 at 13:47
  • @MSalters come on, you're comparing two totally different things here! `operator []` is much more complex than a simple swap operation. I don't think the answer `std::swap` would satisfy a potential employer at an interview, so it's great to know how to do something as simple as that. what's the problem anyway? it's not like it's a **bad** thing (it's not inefficient, not unreadable, not verbose) – Oleksiy Aug 16 '13 at 14:03
  • `operator[](int index)` is basically just `*(base+index)`. Not hard at all. But you're missing the point: zero one infinity. Replacing Standard functions two levels deep is arbitrary. – MSalters Aug 16 '13 at 14:09
0

Works fine, some slight modifications for style and better behavior:

#include <iostream>
#include <cstring>

char* reverse(const char* t);

int main() {
    const char* mystring = "reverse me";
    char* revstring = reverse(mystring);

    std::cout << "Before reversing: " << mystring << std::endl
              << "After reversing: " << revstring << std::endl;

    delete[] revstring;
    return 0;
}

char* reverse(const char* t) {
    size_t j;
    int k;
    char* aString = new char[100];

    for(j = 0, k = std::strlen(t) - 1; j < std::strlen(t); j++, k--) {
        aString[j] = t[k];
    }

    aString[j] = '\0';

    return aString;
}

As others have said, this is fine as an academic exercise, but it's best not to use C strings in C++ if you can avoid it, and it's better to move strlen() out of the loop.

Crowman
  • 25,242
  • 5
  • 48
  • 56