Peace and Love
Peace and Love

Reputation: 11

Recursive function for converting lower case to upper case

I've been tasked to write a recursive function in C that changes a string of any chars to all caps (without using toupper() or any other function for that matter).

This is the previous code and attempt to solve the question:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char* allCapStringRec(char str[])
{
    if (str[0] == '\0' )
        return 0; 

    if (str[0] >= 'a' && str[0] <= 'z')
        str[0] = str[0] - 32;

    return allCapStringRec(str+1);
}

The code needs to compile successfully through a "test" to check the code - The function works by itself but doesn't complete that testing.

The testing:

void Test3(char str[], char expected[], int dec)
{
    char *result = allCapStringRec(str);

    if (strcmp(result, expected) != 0)
        printf("allCapStringRec => Your Output is %s, Expected: %s\n", result, expected);
}

int main()
{
    Test3("123321", "123321", 4);
    Test3("abBba", "ABBBA", 4);
    Test3("ab$cd", "AB$CD", 4);

    printf("done");
    return 0;
}

my output:

dupCapStringRec => Your Output is , Expected: 123321

Sorry for all the edits, it's my first question here. I need help knowing what I'm doing wrong:)

Upvotes: 1

Views: 619

Answers (3)

cdlane
cdlane

Reputation: 41905

This is a terrible use of recursion! That's out of the way, so how can we fix this code:

The other answers, so far, assume you can change Test3(), I'm assuming you can't -- it's a test routine being applied to your code by someone else. If so, I see two ways to deal with it's call:

char *result = allCapStringRec(str);

First, is to assume we can compile this code with writable strings --that is we're allowed to modify any string that gets passed in. In which case, having allCapStringRec() return a value is just a convenience and not an integral feature of it's recursion:

char *allCapStringRec(char str[])
{
    // assumes compiled with -fwritable-strings

    if (str[0] == '\0') {
        return str;
    }

    if (str[0] >= 'a' && str[0] <= 'z') {
        str[0] -= ' ';
    }

    allCapStringRec(str + 1);

    return str;  // convenience return, not strictly necessary
}

If we can't assume writable strings, things get more complicated as we have to create a new string and return that. Here the returning of a string result becomes integral to the recursion:

char *allCapStringRec(char str[])
{
    char *upper = calloc(strlen(str) + 1, sizeof(char));  // calloc initializes to '\0'

    if (str[0] == '\0') {
        return upper;
    } else if (str[0] >= 'a' && str[0] <= 'z') {
        upper[0] = str[0] - ' ';
    } else {
        upper[0] = str[0];
    }

    char *tail = allCapStringRec(str + 1);

    (void) strcpy(upper + 1, tail);

    free(tail);

    return upper;
}

If this is what was assumed, then Test3() is flawed and should free(result) as the last thing it does to avoid a memory leak. But using calloc() and free() violates your "without using toupper() or any other function" requirement so this can't be solved for non-writable strings.

Upvotes: 1

Lundin
Lundin

Reputation: 214395

Problem 1: You attempt to modify read-only string literals. Why do I get a segmentation fault when writing to a "char *s" initialized with a string literal, but not "char s[]"? Instead, you must pass a writable string to the function since you intend to change the string in-place.

Problem 2: if (str[0] >= 'a' && str[0] <= 'z') str[0] = str[0] - 32; isn't really portable code.

Problem 3: recursion for the sake of recursion (and now we have 3 problems).


You can somewhat more efficiently and a lot more portably implement this function as:

(This can also be quite easily modified to cover locale: French, Spanish, German etc)

void str_toupper (char* str)
{
  static const char LUT[127] =
  {
    ['a'] = 'A',
    ['b'] = 'B',
    ['c'] = 'C',
    ['d'] = 'D',
    ['e'] = 'E',
    /* you get the idea */
  };

  for(size_t i=0; str[i] != '\0'; i++)
  {
    char converted = LUT[ str[i] ];
    if(converted)
      str[i] = converted;
  }
}

Usage:

char str[] = "Hello World!";
str_toupper(str);

The trick here is that the look-up table sets all items not used to zero and those are ignored during upper-case conversion.


If you want to use recursion just for the heck of it, then this would be that:

#ifdef MUST_USE_RECURSION_FOR_REASONS_UNKNOWN

void str_toupper (char* str)
{
  static const char LUT[127] =
  {
    ['a'] = 'A',
    ['b'] = 'B',
    ['c'] = 'C',
    ['d'] = 'D',
    ['e'] = 'E',
    /* you get the idea */
  };

  if(*str == '\0')
    return;

  char converted = LUT[*str];
  if(converted)
    *str = converted;  
  str_toupper(str+1); // tail call
}

#endif

Upvotes: 0

Jabberwocky
Jabberwocky

Reputation: 50831

Your code is wrong, but however it's not a total failure.

This needs to be changed:

  • Don't make the allCapStringRec return a pointless pointer but make it a void function.
  • Don't try to modify string literals (they can't be modified), but copy the string literals to a temporary buffer and do the transformation on that buffer. More information about this problem about string literals.

More explanations in the comments:

void allCapStringRec(char str[])  // make the function void
{
  if (str[0] == '\0')
    return;                       // don't return anything

  if (str[0] >= 'a' && str[0] <= 'z')
    str[0] = str[0] - 32;

  allCapStringRec(str + 1);       // don't return anything
}


void Test3(char str[], char expected[], int dec)
{
  char testbuffer[100];           // temporary buffer that can be modified
  strcpy(testbuffer, str);        // copy source to temp buffer
  allCapStringRec(testbuffer);    // convert string in temp buffer

  if (strcmp(testbuffer, expected) != 0)   // compare converted testbuffer with expected
    printf("allCapStringRec => Your Output is %s, Expected: %s\n", testbuffer, expected);
}

Upvotes: 0

Related Questions