C - Extracting capital letters from string

I am working on a program where the goal is to read in a string from a file and print out only the capital letters. My code is below:

#include #include //enables the use of string functions #define BUFFER_SIZE 25 //limits string to 25 characters #define INPUT_FILE "l6p2.txt" //Function prototypes char extractCaps(char *str); int main() < //Create an array that can hold 25 characters char string[BUFFER_SIZE] = < 0 >; //Open the text file FILE *fp = NULL; fp = fopen(INPUT_FILE, "r"); //read only //Scan string from the file fscanf(fp, "%s", string); //Call the function and pass the string from the file as the parameter extractCaps(string); printf("%s", string); return 0; > //Recursive function that will extract capital letters from a string char extractCaps(char *str) < static int i=0; if(i < strlen(str)) < //Base case (if the string is empty) if (str[i] == '\0') // \0 is a character that is automatically added at the end of a string < return; >else if((str[i] >= 65 && str[i] else //if not a capital letter < //Recursive call. move to the next letter and repeat the above steps i++; return extractCaps(str); >> printf("\n"); > 

The l6p2.txt file contains the string "hHeElLlLoO" . As such, I am expecting the output to be "HELLO" , but my code here is still outputting the entire string, "hHeElLlLoO" . I originally had the code working when I had the printf statement inside of my extractCaps function, but I was told to change it so that the printing occurs in main . I, for the life of me, cannot understand why this code is behaving incorrectly. The logic makes sense to me, but then again I hardly know squat about programming. As a side note, I've heard that combining static variables with recursion is a bad idea, so I tried using i as an argument to extractCaps but still got the same (incorrect) output. Where am I going wrong? EDIT: It is a requirement that extractCaps be a recursive function.

There are really too many problems here to enumerate. The first thing I would do is learn about loops. Don't use recursion here. Beyond that, your function returns information but the calling code doesn't do anything with what gets returned.

The extractCaps function never actually changes the contents of the input string buffer. So of course the original string is still in there after calling that function and the printf shows that accordingly.

Step 1 is to fix the code so that is compiles without any warnings or errors. The code you posted doesn't even compile, so I don't know how you ran it.

Yes, my apologies. I used recursion because I am required to do so. I could easily code the same problem using loops, but this is not allowed.

In that case, I would drop the class. C'mon, there's gotta be a simple task where recursion is actually useful, like computing a greatest common divisor. Extracting capital letters is three lines of code, a for loop, an if statement, and a printf .

3 Answers 3

While you already have your answer, here are a few things to think about going forward. First, when you open a file for reading always validate that the opening actually succeeded:

/* Open the text file AND VALIDATE */ FILE *fp = NULL; if (!(fp = fopen ("filename.txt", "r")))

Instead of hard-coding "filename.txt" in your code, you can make your code much more flexible if you simply pass the filename as an argument to your program. That is the purpose of the int main (int argc, char **argv) parameters to main . It takes little additional effort:

int main (int argc, char **argv) < . /* validate sufficient input on command line (filename) given */ if (argc < 2) < fprintf (stderr, "error: insufficient input. usage: %s filename", argv[0]); return 1; >/* Open the text file AND VALIDATE */ FILE *fp = NULL; if (!(fp = fopen (argv[1], "r"))) < fprintf (stderr, "error: file open failed '%s'.\n", argv[1]); return 1; >. 

While you can read a string with fscanf , there are many pitfalls involved in trying to do input in general with the Xscanf family of functions. (they have their place, but for general reading, there are better options)

When reading a file, you generally want to consider using one of the line-oriented input functions provided by the C-standard library like fgets or getline . The general approach is to read a line at a time into a buffer, and then parse the contents of the buffer to obtain the information you need. It is no more difficult to read a line at a time than it is to read a string at a time, but it is a lot less prone to error e.g.:

/* read each line in file into buf * BUFFER_SIZE set to 256 (or as needed) */ while (fgets (buf, BUFFER_SIZE, fp)) extractCaps (string, buf, &stridx); 

In your case here, you need to somehow collect all the capital letters into a second buffer, so they are saved for printing after your file read is done. Granted, with a one-line file, there is little need, but since these are just tips, let's consider if your file was more than one line, e.g.:

hHeElL lLoO 

What then? In this case, you would read a line, and then pass that line to extractCaps. extractCaps would then scan the line, either recursively as you have, or simply stepping through each character with a pointer, test each character to determine if it were a capital, then save the character in the string, and increment the string index by 1 (so it would know where to save the next one). By using a separate buffer to hold the capital letters, you can handle an input file having any number of lines.

Whenever you are filling any type buffer of fixed size, you need to check if the buffer is full, and take the proper action if it is. (writing beyond the end of the buffer is a sure way to run into trouble.)

Lastly, you would need a way to keep track of the index for where to write the next Capital in the buffer between calls to extractCaps. There are several ways. You can have your function return the index number and keep track of it that way --or-- you can pass a pointer to the index as an argument to extractCaps so that the value updated in extractCaps in available back in main . Putting all of those pieces together in an alternate extractCaps function, it might look like:

/* simple function to extract capital letters from 'buf' and save in 'str' updating the index 'idx' to reflect the number of chars in string */ void extractCaps (char *str, char *buf, size_t *idx) < if (!buf || !str || !idx) < /* validate parameters */ fprintf (stderr, "extractCaps() error: invalid parameter.\n"); return; >char *p = buf; /* pointer to buf */ while (*p) < /* for each char in buf */ if ('A' str[(*idx)++] = *p; /* copy CAP to str, increment idx */ > p++; > > 

Taking the next step and putting all the pieces together, here is another way to approach your problem. This is just something to consider for the future, use it for comparison purposes, and as an example of reading a file in a line-oriented fashion:

#include #include #define BUFFER_SIZE 256 /* limits line to 256 characters */ /* Function prototypes */ void extractCaps (char *str, char *buf, size_t *idx); int main (int argc, char **argv) < /* Create an array that can hold 25 characters */ char buf[BUFFER_SIZE] = < 0 >; char string[BUFFER_SIZE] = < 0 >; size_t stridx = 0; /* validate sufficient input on command line (filename) given */ if (argc < 2) < fprintf (stderr, "error: insufficient input. usage: %s filename", argv[0]); return 1; >/* Open the text file AND VALIDATE */ FILE *fp = NULL; if (!(fp = fopen (argv[1], "r"))) < fprintf (stderr, "error: file open failed '%s'.\n", argv[1]); return 1; >/* read each line in file into buf */ while (fgets (buf, BUFFER_SIZE, fp)) extractCaps (string, buf, &stridx); /* print the results in string */ printf("\nAll caps in file : %s\n\n", string); return 0; > /* simple function to extract capital letters from 'buf' and save in 'str' updating the index 'idx' to reflect the number of chars in string */ void extractCaps (char *str, char *buf, size_t *idx) < if (!buf || !str || !idx) < /* validate parameters */ fprintf (stderr, "extractCaps() error: invalid parameter.\n"); return; >char *p = buf; /* pointer to buf */ while (*p) < /* for each char in buf */ if ('A' str[(*idx)++] = *p; /* copy CAP to str, increment idx */ > p++; > > 

Input File

$ cat dat/hellocaps.txt hHeElLlLoO 

Use/Output

$ ./bin/extract_caps dat/hellocaps.txt All caps in file : HELLO