current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 10 down vote favorite
1

I had this interview question like a year ago and was asked to code, on a piece of paper, how to reverse each word of a string. Since I am used to Java, I proposed the obvious answer of using split + reverse, which are native commands in Java. I was then told I couldn't use those, so I floundered and ended up with a really terrible solution (even though it technically would've worked).

Anyway, it was bugging me lately, so I gave it a shot in straight C, which I am not very good at, so it took me a good while to actually get it working.

I was wondering:

  1. Is this a good solution?
  2. Have I forgotten anything obvious?
  3. Have I done anything non-kosher in the C world?

Again, I'm not very good at C, so even small points will probably help me out.

#include <stdio.h>

void reverseString(char* start, char* end){
   while (start < end){
      char temp = *start;
      *start = *end;
      *end = temp;
      ++start;
      --end;
   }   
}

char* word_start_index(char* p)
{
    while((*p != '\0') && (*p == ' ')){
        ++p;    
    }

    if(*p == '\0')
        return NULL;
    else
        return p;
}

char* word_end_index(char* p)
{
    while((*p != '\0') && (*p != ' ')){
        ++p;
    }

    return p-1;
} 

void main(){
    char arr[] = "kevin is a good programmer";
    char* test = arr;

    while (test != '\0'){
        char* curWordStart = word_start_index(test);
        if (curWordStart == NULL)
            break;
        char* curWordEnd = word_end_index(curWordStart);
        reverseString(curWordStart, curWordEnd); 
        test = curWordEnd + 1;
    }
    printf("%s \n", arr);
}

Also, would taking a different approach, like in higher level languages of breaking the string into an array of strings (so I guess a 2D array of chars), then stepping through and reversing each one, be a good approach as well? I thought about this first and was unable to hash it out.

share|improve this question
1  
Another thing that you need to consider is that words are not always terminated by a space. Punctuation also counts!.:,;? And, being awkward, what about numbers? –  Glenn Rogers Nov 5 '12 at 12:19

4 Answers 4

up vote 4 down vote accepted

I have a few issues with in your code:

  • Consistency in naming - use either camelCase or not_camel_case but don't mix

  • consistency in braces. The opening brace for a function goes in the first column.

  • word_start_index and word_end_index should take a const parameter

  • word_start_index is the same as strspn(string, " "); or if you are also looking for punctuation, strspn(string, " \t\n.,;:");

  • word_end_index - as for word_start_index but use strcspn (note the 'c')

  • word_end_index as a function (ie not in your context) fails for an empty string or a string starting with a space (it returns the char before the string starts).

  • variable test in main() is misnamed. I would prefer something that shows it is a string.

  • the test while (test != '\0') in main() is wrong - should be *test != '\0'. Your loop always exits from the break

  • no return or parameters in main()

Also, arguably the position of the stars in your pointers is wrong. I prefer char* p to be written char *p, which makes it clear that it is p that takes the star. Consider code such as char* a, b;. This is bad because it gives the impression that b is a pointer.

share|improve this answer
    
Thanks for the comments! Do you think taking a different approach like in higher level languages of breaking the string into an array of string (so I guess a 2-d array of chars), then stepping through and reversing each one would be good approach as well? I thought about this first and was unable to hash it out. –  KDiTraglia Nov 5 '12 at 15:11
    
No, your approach is fine –  William Morris Nov 5 '12 at 15:16
    
OK, thanks for the details, I was definitely unclear about how some of the pointers need to be declared and compared and had some trial and error there. The camel case issues were just a result of copy pasting a bit off the web, but again thanks for the help. –  KDiTraglia Nov 5 '12 at 15:18
up vote 4 down vote

I like it; nice and logical and easy to follow.

The only change I would make is the test for space.

*p == ' '

I would replace this with

isspace(*p)
share|improve this answer
up vote 2 down vote

I also met this question during a interview several years before, the string was null-terminated and separated by spaces. my idea was same as yours, just save some lines of code, comments in line.

// Reverse the characters between pointer p and q
void ReverseWord(char* p, char* q)
{
    while(p < q)
    {
        char t = *p ;
        *p++   = *q ;
        *q--   = t ;
    }
}


// Reverse all words in a sentence.
void ReverseSentence(char *s)
{
    char *p = s ;   // point to the start position of a word
    char *q = s ;   // point to the end position of a word(white space or '\0')

    while(*q != '\0') // While string not ends
    {
        if (*q == ' ') // Get a word?
        {
            ReverseWord(p, q - 1) ;
            q++ ; // move to next word
            p = q ;
        }
        else
            q++ ;
    }

    ReverseWord(p, q - 1) ; // Reverse the last word
}
share|improve this answer
up vote 1 down vote

Mine is also very similiar:

#include <stdio.h>
#include <ctype.h>

void reverse(char *p, char *q)
{
    while (p<q)
    {
        char t = *p;
        *p++   = *q;
        *q--   =  t;
    }
}

char *reverse_each_word(char* str)
{
    char *p, *q = str;
    while(*q)
    {
        p = q; while(*p && !isalnum(*p)) p++; // Skip non-word chars
        q = p; while(*q &&  isalnum(*q)) q++; // Skip     word chars
        if(*p) reverse(p, q-1);
    }    
    return str;
}

int main()
{
    char str[] = "An answer on http://codereview.stackexchange.com/questions/18229";
    printf("%s\n", str);
    printf("%s\n", reverse_each_word(str));    
    return 0;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.