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.

This reverses the case of a character in a string (a string being an array of characters). How does it look? Anything I can improve on? Besides documentation, in particular, any thoughts on the type casting found in str_case_rev?

#include <stdio.h>  // fprintf
#include <stdlib.h> // malloc
#include <string.h> // strlen
#include <ctype.h>  // toupper, tolower

/*
 * a string is an array of characters, in C, all arrays
 * are always passed never passed value, always a pointer to
 * the variable
 * This is why the caller does not need to call the function like:
 * camel_case_rev(src, &dest, len)
 *
 * Since here the array of characters ("a string") is already being
 * passed as a pointer value
 */
void str_case_rev(const char *src, char *dest, size_t len)
{
        size_t i = 0;

        for(; i < len; i++)
        {
                if(src[i] <= 'Z' && src[i] >= 'A')
                {
                        *(dest + i) = (char)tolower((int)src[i]);
                }
                else if(src[i] <= 'z' && src[i] >= 'a')
                {
                        *(dest + i) = (char)toupper((int)src[i]);
                }
                else
                {
                        *(dest + i) = src[i];
                }
        }

        i++;
        *(dest + i) = '\0';
}

int main(int argc, char **argv)
{
        if(argc < 2)
        {
                fprintf(stderr, "Usage: %s <string>\n", argv[0]);
                return EXIT_FAILURE;
        }

        char *dest = NULL;

        size_t len = strlen(argv[1]);

        dest = malloc(len + 1);

        if(NULL == dest)
        {
                fprintf(stderr, "Memory error\n");
                return EXIT_FAILURE;
        }

        str_case_rev(argv[1], dest, len);

        fprintf(stdout, "%s\n", dest);

        free(dest);

        return EXIT_SUCCESS;
}
share|improve this question

3 Answers 3

The case determination code (src[i] <= 'Z' && src[i] >= 'A') doesn't look right (think about locales). isupper and islower are specifically designed to deal with that. Besides, you are already using toupper and tolower.

share|improve this answer

Rather than have a big conversion statement I would just do:

 static const char convert[] = {
                          0, 1 ,2 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15
                          ....
                          /* The character code for 'A' 65 replace with 'a' 97 */

                          62, 63, 64, 'a', 'b', 'c', 'd',
                          ....
                          /* The character code for 'a' 97 replace with 65 'A' */
                          94, 95, 96, 'A', 'B', 'C', 'D', 

                          ....
                          248, 249, 250, 251, 252, 253, 254, 255};

 void str_case_rev(const unsigned char *src, char *dest, size_t len)
 {
     for(; i < len; i++)
     {
         dest[len] = convert[src[len]];
     }
     dest[len] = '\0';
 }

Then all you have to do is define the conversion in a single array.
Since a char is only 8 bits you only need to define an array of 255 characters (just make sure you use unsigned char).

share|improve this answer
    
Can you elaborate on the array of 255 characters. Admittedly, I don't have a lot of experience with bit shifting. –  true 21 hours ago

I would call a function to convert uppercase to lowercase and vice versa something like str_case_toggle().

I am puzzled by the len parameter in the function signature. Usually, C strings are null-terminated. Since this function only makes sense on text strings and not binary data, I think that it makes more sense to look for the null terminator in src than to state the length explicitly.

On the other hand, sometimes functions accept a bufsize parameter, which has subtly different semantics. A bufsize indicates the size of the destination buffer and helps to prevent buffer overflows. However, in that case, I would still expect the the loop to stop if it encounters a NUL byte in src first, so as not to try to read possibly invalid memory.

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.