Reputation: 11
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
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
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
Reputation: 50831
Your code is wrong, but however it's not a total failure.
This needs to be changed:
allCapStringRec
return a pointless pointer but make it a void
function.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