0

When I compile this program I only get the first capital letter but not the rest.

Input:

ABldjfdslkjfCK

I only get 'A' that is it?

#include <stdio.h> #include <string.h> FILE *fp; int main(void) { int size; char input[100]; // array size of 100 if (fp = fopen("message.txt","r")) // file exists { fgets(input,100,fp);// scans the sentence. } else { printf("file not found");// if there is no such a file. } size=strlen(input); recursive(size,input); return 0; } int recursive(int size, char array[]) { static int index = 0; // static so when the function is called, the value is kept if (index < size) // start from index 0 until the size-1 { if (array[index] >= 'A' && array[index] <= 'Z') // check for A to Z (CAPITALIZE CHARACTERS only) { printf("%c\n", array[index]); // print it } else { return recursive(size,array); // calls the function (recursion) } } return 0; }
Improve this question
7
  • 4
    You should not have a global FILE*. You should declare it in main(). Commented Nov 8, 2009 at 20:18
  • 1
    fgets(input,100,fp) -> fgets(input,sizeof(input),fp) Commented Nov 8, 2009 at 20:22
  • 1
    Use 'isupper()' from <ctype.h> to detect upper-case letters. Commented Nov 8, 2009 at 22:08
  • 2
    You really should not be using a static variable 'index' in a recursive function; unless you are extremely careful, it means you can only use the function once (because if it is not zeroed when the function exits from the call stack, all hell is likely to break loose, especially if the previous line was long and the current line is short, and they don't share the same input buffer). [After reading more answers, I see Michael Burr also noted this problem.] Commented Nov 8, 2009 at 22:16
  • 2
    An additional problem is that if you fail to open the file, you still try to process the string which was not explicitly initialized by reading any data into it - which can lead to trouble. The 'strlen()' and 'recursive()' functions (and fclose() - its absence is another problem) should be in the 'if' clause. I would also argue that errors should be written to stderr, not stdout. Commented Nov 8, 2009 at 22:18

6 Answers 6

Reset to default
14

You never increment the value of index. Furthermore, you do not call the function recursive if the current character is a capital letter, so the function just returns.

Rather than using a static variable for index, it would be better to pass it in as an argument to recursive; otherwise, the function is non-reentrant.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Reentrant - en.wikipedia.org/wiki/Reentrant_%28subroutine%29 (for anyone who doesn't remember what it means, like me :)
4

your recursive function calls to itself only if it finds a non capital character. when it finds the first capital character it prints it and quits

Improve this answer

Comments

3

Among the other issues that your recursive function has is that the index variable is static. That's not a problem in the current versions, since you don't actually use it except in a trivial way. But once you try to fix the other problems (which may result in you using index in s more complicated way), having it static will pose a couple problems:

  • there's no good way to initialize it properly after the first use. This is often fixed by having a non-recursive 'wrapper' function that initializes the state variable and passes it to a private recursive function that does the actual work (and takes the value as a parameter instead of using a static instance). The wrapper just kicks off the function that does the real work.
  • the recursive calls to the function will modify the static variable, which will modify the state of the 'saved' instance of the in progress recursive calls. This might not be a problem if the recursion is a type known as 'tail recursion' where the caller that performs the recursive call won't perform any additional work after the recursive call returns, but not all recursive functions are tail recursion. Though your example could easily be (I'd still try to avoid the static variable, though).
Improve this answer

Comments

3

Your current function prints only A because as soon as it finds an uppercase letter (A in your case) it returns a 0.

There are other issues too, so I would rewrite the function like this:

#include <ctype.h> /* for isupper */ void recursive(const char* s) { /* stop condition: empty string */ if (s[0] == '\0') return; /* main body: print letter if capital */ if (isupper(s[0])) printf("%c\n", s[0]); /* recursion: advance to the next character */ recursive(s + 1); }

Use it like this: recursive(input).

Improve this answer

1 Comment

Of course, this is an example of tail recursion, which is trivially removed by using a while loop instead - but you've captured the intent rather clearly (and the basic rule of successful recursion: know when to stop and when to continue).
2

Two problems:

  1. You are terminating your recursion incorrectly. You need to keep recursing until you reach the end of the array.
  2. You have a second problem where you are not incrementing index so each call will always look at the first character.
Improve this answer

Comments

1

The first error I see is that you never increment index.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.