0

This is the problem I'm trying to solve:

Input:
First line contains N, the size of the string. Second line contains the letters (only lowercase).

Output:
Print YES if all vowels are found in the string, NO otherwise.

Constraints:
The size of the string will not be greater than 10,000. 1 ≤ N ≤ 10000

The following code I wrote is always showing NO.

 #include <stdio.h>
 #include<conio.h>

 int main()
 {
   int a,b,c=0,d=0,e=0,f=0,g=0,i;
   char string[10000];
   scanf("%d",&a);
   scanf("%s",string);
   for(i=0;i<a;a++)
   {
     if(string[i]==('a'))
       c=1;
     if(string[i]==('e'))
       d=1;
     if(string[i]==('i'))
       e=1;
     if(string[i]==('o'))
       f=1;
     if(string[i]==('u'))
       g=1; 
   }
   if((c==1)&&(d==1)&&(e==1)&&(f==1)&&(g==1))
     printf("YES");
   else
     printf("NO");
   return 0;
   getch ();
 }
Improve this question
8
  • 6
    It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: How to debug small programs Commented Aug 28, 2015 at 14:41
  • 1
    Protip: With the exception of things like loop counters, etc you may want to name your variables so that their name documents their purpose. This will make it easier to spot bugs and allow you to understand your code as its complexity grows, helping ensure that you remain sane. Commented Aug 28, 2015 at 14:44
  • 3
    in your for loop for(i=0;i<a;a++) you increment a not i Commented Aug 28, 2015 at 14:46
  • 2
    As already stated: Use a debugger. Then, name your variables, and if you expect someone including your future self to read that code format it properly. Finally, C and C++ are different languages. Your code looks like C, which is why I remove the C++ tag. (Why do you use conio.h? Better don't.) Commented Aug 28, 2015 at 14:49
  • for(i=0;i<a;i++) would be right Commented Aug 28, 2015 at 14:51

3 Answers 3

Reset to default
2

Here is an infinite loop that causes a problem:

for(i=0;i<a;a++)

You should increment i, instead of a (length of a string). If you fix this one char in loop statement, the program will run well at all. Anyway, I changed your code a bit to be more readable. Take a look if you want, just for your information, sir:

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

 int main()
 {
   int len, a=0, e=0, i=0, o=0, u=0, it;
   char string[10000];

   scanf("%d", &len);
   scanf("%s", string);

   for(it=0;it<len;it++)
   {
     if(string[it]=='a') a = 1;
     else if(string[it]=='e') e = 1;
     else if(string[it]=='i') i = 1;
     else if(string[it]=='o') o = 1;
     else if(string[it]=='u') u = 1;
   }
   if(a && e && i && o && u) printf("YES\n");
   else printf("NO\n");

   system("PAUSE");
   return 0;
 }

I assume you are running your program under Windows, so instead of conio's getch() try to use system("PAUSE") or just even better way to do this (for both Windows for UNIX): getchar()

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

9 Comments

May also be worth mentioning the strstr or strchr functions as well.
Of course strchr may do its job here, but is it really needed, sir? There is nothing work about comparing with == unless it is not comparing strings, right?
I'm not saying change your code, but it is always nice in an answer to mention the functions that are part of the standard C library that do, in part, what the OP is attempting to do.
And instead of system("PAUSE"), use getchar()... Nobody said this was for Windows and the system call in general should be avoided.
@MateuszKwaśniak Actually, conio.h rather suggests MS DOS :)
|
1

I've renamed all of your variables, but otherwise left the code the same.

 #include <stdio.h>
 #include<conio.h>

 int main()
 {
   int foundA = 0, foundE = 0, foundI = 0, foundO = 0, foundU = 0;
   int i, length;
   char string[10000];

   scanf("%d", &length);
   scanf("%s", string);

   for(i=0; i<length; length++)
   {
     if(string[i]==('a'))
       foundA=1;
     else if(string[i]==('e'))
       foundE=1;
     else if(string[i]==('i'))
       foundI=1;
     else if(string[i]==('o'))
       foundO=1;
     else if(string[i]==('u'))
       foundU=1; 
   }
   if((foundA==1)&&(foundE==1)&&(foundI==1)&&(foundO==1)&&(foundU==1))
     printf("YES");
   else
     printf("NO");
   return 0;

   getch ();
 }

Looking the the for-loop condition for(i=0; i<length; length++), I think it's pretty clear what's wrong. Instead of incrementing the counter, you're incrementing the length of the string. Eventually, the counter overflows to a negative number, and the loop terminates without ever looking at a character besides the first one. The lesson here is to name your variables properly.

If you want to be picky, then signed integer overflow is undefined behavior, but for most systems, INT_MAX + 1 will be INT_MIN.

Improve this answer

Comments

1

This program can be done in more simpler way other as below.

#include <stdio.h>
#include<conio.h>

int main()
{
    int i, flag = 0;
    char string[10000], *ptr;
    char cmp[] = "aeiou";

    printf("Please enter string = " );
    scanf("%s", string);
    i = 0;
    while(cmp[i])
    {
        ptr = string;
        while(*ptr)
        {
            if(cmp[i] == *ptr)
            break;
            ptr++;
        }
        if(*ptr != cmp[i++])
        {
            flag = 1;
            break;
        }
    }

    if(flag == 1)
        printf("NO");
    else
        printf("YES");
}

In this program I have used just one flag instead of 5 flags. Always try to write simple code rather then using unnecessary variable and flags.

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.