Any issues with the following in-place reverse function?

Question

I wrote this following in-place reverse function and it works fine.

However, when I googled for a solution, I found plethora of more complex solutions but nothing this simple. Will the following not work for certain inputs or has some performance issues?

void reverse(char* str) {
    int n = strlen(str);
    char temp;

    for (int i=0; i<n/2; i++) {
        temp = str[i];
        str[i] = str[n-i-1];
        str[n-i-1] = temp;
    }
}

int main() {
    char input[] = "Reverse Me!";
    reverse(input);
    return 0;
}

This task is often given to new programmers, which might explain why there are a lot of bad solutions around — M.M, Aug 21 '15 at 01:32
str could be not null terminated, in which case strlen fails. — 4nt, Aug 21 '15 at 01:34
FWIW I've been writing C for 12 years and I'd never have thought of using your /2 solution (instead I'd allocate a new buffer and iterate the string backward to fill it). Your solution is awesome, it'd just never have occurred to me. — James M, Aug 21 '15 at 01:49
To say the somewhat obvious, unicode characters won't work there. — Quill, Aug 21 '15 at 02:15

autistic · Answer 1 · 2015-08-21T02:31:39.510

3

Yes. This may work correctly on your system; however, strlen returns size_t, which might have a greater precision than int, resulting in only a part (or none?) of the actual string being reversed. That's an easy fix: Declare n as a size_t instead of an int.

Your solution could be simpler, if you were to decrease n each iteration while you're increasing x. Then you wouldn't need as much of the subtraction logic, or any of the division logic.

void reverse(char *str) {
    for (size_t x = 0, y = strlen(str); y --> x; x++) {
        char temp = str[x];
        str[x] = str[y];
        str[y] = temp;
    }
}

edited Aug 21 '15 at 02:31

answered Aug 21 '15 at 02:15

autistic

1
3
35
80

Less obfuscated as `y-- > x` – chux - Reinstate Monica Aug 21 '15 at 02:24
@MattMcNabb That's the [goes to](http://stackoverflow.com/questions/1642028/what-is-the-name-of-the-operator) operator ;) – autistic Aug 21 '15 at 02:26
3

Say, what is that "-->" operator you have there?? – M.M Aug 21 '15 at 02:27
1

@MattMcNabb Correct! You've won Jeopardy! Your prize is an up-vote! – autistic Aug 21 '15 at 02:34

score 0 · Answer 2 · answered Aug 21 '15 at 01:32

0

No, there are no issues with your code. It will work perfectly in any situation with ASCII characters.

answered Aug 21 '15 at 01:32

user156213

736
4
12

Gene · Answer 3 · 2015-08-22T16:19:42.857

@Rahat Mahbub is right about size_t. On many 64 bit machines, your algorithm will fail for strings 2^31 in length or longer. Using size_t will prevent that.

But the main problem is the complexity. A clearer code is something like:

void reverse(char *str) {
  if (*p == '\0') return;
  for (size_t i = 0, j = strlen(str) - 1; i < j; ++i, --j) {
    char tmp = str[i]; 
    str[i] = str[j]; 
    str[j] = tmp;
  }
}

In words, place i at the start of the string and j at the end. Swap characters they index and move both toward the middle of until they touch.

With pointers, it's even a bit more succinct:

void reverse(char *p) {
  if (*p == '\0') return;
  for (char *q = p + (strlen(p) - 1); p < q; ++p, --q) {
    char tmp = *p; 
    *p = *q; 
    *q = tmp;
  }
}

Unfortunately as @chux pointed out, you need the if or some other footwork to avoid computing q = p - 1 on empty input, which is not a valid pointer. In the indexed version, you need it because size_t is an unsigned type; it has no -1 value.

I too prefer indices over pointer arithmetic,yet `j = strlen("") - 1; i < j` remains a problem as it becomes `i < SIZE_MAX`. "So other may dream 2017" — chux - Reinstate Monica, Aug 22 '15 at 02:41

Any issues with the following in-place reverse function?

3 Answers3