0

I wrote a program to find the longest word in a string and print the number of letters in the longest word. But the code is not printing. I analyzed the program many times but I could not find the solution.

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

int main() {
    char string[100] = "Hello Kurnool";
    int i = 0, letters = 0, longest = 0;

start:

    for (; string[i] !=' '; i++) {
        letters++;  
    }

    if (letters >= longest)
        longest = letters;

    if (string[i] == ' ') {
        letters = 0;
        i++;
        goto start;
    }

    printf("%d", longest);

    return 0;
} 
Uwe Keim
  • 39,551
  • 56
  • 175
  • 291
  • 9
    Read [how to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). Compile with all warnings and debug info: `gcc -Wall -Wextra -g` with [GCC](http://gcc.gnu.org/). Improve your code to get no warnings. [use the `gdb` debugger](https://sourceware.org/gdb/current/onlinedocs/gdb/). Be aware that *stdout* is buffered (so use `fflush` or end every `printf` format string with `\n`...) – Basile Starynkevitch Jun 15 '18 at 09:03
  • 16
  • printf() out the value of 'i' in the for loop. – Martin James Jun 15 '18 at 09:06
  • 6
    You don't check for end of string change the test `for(;string[i]!=' ' && string[i]!=0;i++)` – Frankie_C Jun 15 '18 at 09:06
  • Why should not I use goto –  Jun 15 '18 at 09:06
  • 1
    ..or put a breakpoint on 'letters++;' and inspect 'i'. – Martin James Jun 15 '18 at 09:07
  • 6
    Why avoiding `goto`: Because Dijkstra [told us](https://homepages.cwi.nl/~storm/teaching/reader/Dijkstra68.pdf) in 1968 – Basile Starynkevitch Jun 15 '18 at 09:08
  • 6
    Hey guys, have you heard that goto is considered harmful!? Lets debate it, surely that's not been done before. – Lundin Jun 15 '18 at 09:30
  • 2
    why not goto? because enternal loop is much more clear, and this question probably never be set – Jacek Cz Jun 15 '18 at 09:34
  • Never use go to and similar things, that all can be handled by functions / recursions / loops, its more readable and easier to debug.. Except this- you can use [strtok()](https://stackoverflow.com/a/9210560/4892907) to split String by delimiter, then you can have temp variable with the current longest word, and you can get length of each word by using [strlen](https://www.gnu.org/software/libc/manual/html_node/String-Length.html).. So 1. split, 2. loop per number of elements/ words 3. compare to get the longest one – xxxvodnikxxx Jun 15 '18 at 09:44
  • `goto` should only be used in cases where it does improve readability [What is wrong with using goto?](https://stackoverflow.com/q/3517726/995714), [Use GOTO or not?](https://stackoverflow.com/q/379172/995714), [GOTO still considered harmful?](https://stackoverflow.com/q/46586/995714) – phuclv Jun 15 '18 at 10:30
  • 2
    Avoid `goto` in code to avoid lengthy `goto` discussions in code review. – chux - Reinstate Monica Jun 15 '18 at 12:23

5 Answers5

8

Using goto is highly discouraged. You should convert your code to use a loop.

The main problem in your code is you do not stop the scan when you reach the end of the string.

Here is a modified version:

#include <stdio.h>

int main() {
    char string[100] = "Hello Kurnool";
    int i, letters, longest = 0, longest_pos = 0;

    for (i = 0; string[i] != '\0'; i++) {
        for (letters = 0; string[i] != '\0' && string[i] != ' '; i++) {
            letters++;  
        }
        if (letters > longest) {
            longest = letters;
            longest_pos = i - longest;
        }
    }    
    printf("longest word: %d letters, '%.*s'\n",
           longest, longest, string + longest_pos);

    return 0;
}

Note that the implementation can be simplified into a single loop:

#include <stdio.h>

int main() {
    char string[100] = "Hello Kurnool";
    int i, start = 0, longest = 0, longest_pos = 0;

    for (i = 0; string[i] != '\0'; i++) {
        if (string[i] == ' ') {
            start = i + 1;
        } else {
            if (i - start > longest) {
                longest = i - start;
                longest_pos = start;
            }
        }
    }    
    printf("longest word: %d letters, '%.*s'\n",
           longest, longest, string + longest_pos);

    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • You have missed putting braces `{}` in `if (letters > longest)` block because of which the program may not give correct output for other cases. The statements `longest = letters;` and `longest_pos = i - longest;` should be within if block `{` and `}`. – H.S. Jun 15 '18 at 10:08
  • 1
2

Below is my approach. You should use C's string manipulation functions. This is the correct way to deal with strings in C.

In the code below, first I acquire the required bytes to store the input string in heap. Then I use strtok to split the string into tokens based on a delemeter and get the length of each sub string. Finally I free the space that I have allocated with malloc.

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

#define phrase "Hello Kurnool"
int main()
{
    char* string = malloc(strlen(phrase)+1);
    strcpy(string,phrase);
    int longest=0;
    char *token;
    char delimeter[2] = " ";

   /* get the first token */
   token = strtok(string, delimeter);

   /* walk through other tokens */
    while( token != NULL ) {
        printf( " %s\n", token );
        if(longest < strlen(token)){
            longest = strlen(token);
        }
        token = strtok(NULL, delimeter);
   }
    printf("%d",longest);
    free(string);
    return 0;
}
leopal
  • 4,711
  • 1
  • 25
  • 35
1

People say - dont use goto but there is nothing inherently wrong with goto. Only thing is if goto is not used judiciously, it makes code more difficult to understand and maintain. For example, the way you have used it in your program ( instead of goto, a loop is perfect fit in such cases). Check this:
To use goto or not?
What is wrong with using goto?

Coming to your code, the for loop condition does not have check for terminating null character

for (; string[i] !=' '; i++) {

Hence it will not stop at the end of string.
To find the number of letters in longest word of string, you can do:

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

int main() {
    char string[100] = "Hello Kurnool";
    int i, letters = 0, longest = 0;

    for (i = 0; string[i] != '\0'; i++) {
        if (string[i] != ' ') {
            letters++;
            if (letters > longest) {
                longest = letters;
            }
        } else {
            letters = 0;
        }
    }

    printf("longest : %d\n", longest);

    return 0;
}
H.S.
  • 11,654
  • 2
  • 15
  • 32
0

First of all,Please avoid using Goto, it is not a good practice.

Secondly, your loop will run infinite times when it iterates the second time because:

for(;string[i]!=' ';i++) // Here String[i] will never be equal to ' ' As there is no white space after your last word.
amrender singh
  • 7,949
  • 3
  • 22
  • 28
0

You can never expect what might be going wrong with your program if you are using

goto statement

which is never advisable to use rather it's bad programming if you use it. Secondly it looks like you are stuck in an infinite loop so her is a solution to your problem:

#include<stdio.h>
#include<string.h>
void main()
{
    char s[1000];
    scanf("%s",s);
    int i=0;
    int letters;
    int longest=0;
    while(s[i]!=NULL)
   {
      if(s[i]==' ')
      {
         if(longest>=letters)
              {longest=letters;}
         letters=0;
       }
      else
         {letters++;}
    }
printf("%d\n",longest);
}

So, what I have done is assuming a string s which is the input given by the user. You itterate through s till the last input given by the user after which it encounters a NULL character. Now you are searching for the length of the longest word, so you create a variable letters for counting the no. of letters in each word of the string. And if the string s encounters a space indicating the end of a word, then you check if the variable longest is greater than or less than the word count. And again you initialize letters to 0, so that it can start counting the next word from 0 again.So, by this method at the end i.e. after the while loop terminates we get our required output which is stored in the variable longest. So, I guess this will print the no. of letters in the longest word.

FLASH
  • 76
  • 2