Thread: Longest word

You definitely have the general idea right, but there are a lot of things wrong with your code:

• Your indentation is horrible. If it's hard to read the code, it's hard to find mistakes, but easy to make them. Pick one of the first 3 styles listed here: Indent style - Wikipedia, the free encyclopedia.
• You correctly declared main to return an int, but you never return an int. Put a return 0; at the end of main. Also, you should use return 8; instead of calling exit().
• Try to avoid stuff like system("pause"), it makes it hard for us non-Windows people to help you. Besides, there are better solutions: FAQ > Stop my Windows Console from disappearing everytime I run my program? - Cprogramming.com.
• Don't use feof to control your while loop. Read this: FAQ > Why it's bad to use feof() to control a loop - Cprogramming.com. Then, fix your loop control to use the return value of fgets.
• fgets ensures that it will put a '\0' in there, so there's no need to use 199 instead of 200. But better yet, make it so if you change the size of str, you don't need to change the fgets call: fgets(str, sizeof(str), in_file).
• Make word, longest_word and str the same size. If your .dat file is one word per line (as your code suggests), and the biggest word is 20 chars, then str only needs to be 22 chars (to hold the new line and null char), and word and longest_word should be 21 chars (to hold the null terminator). This will help guard against buffer overflows a little bit.
• Don't use magic numbers for checking if a character is a letter. Those numbers are cryptic, and they're specific to ASCII, which isn't used everywhere. #include <ctype.h> and use the isalpha() function. It makes sense and it's portable.
• You never null terminate word, so the strlen might fail.
• Your strcpy is backwards. You're overwriting word with the contents of longest_word. It should be strcpy(longest_word, word).
• You need to initialize longest_word to an empty string (set it's first char to '\0'), otherwise it has garbage, and strlen(longest_word) could be huge, meaning you never find a longer word in your .dat file.
• If your .dat file is one word per line, there's an easier way than what you're doing. After fgets, trim the new line from str and just use strlen(str). See below for an example.

That's all I got in my quick review. Should be enough to keep you going for a while.

Code:

char *p;
p = strchr(str, '\n');
if (p)
    *p = '\0';  // If str has a newline, "trim" it, i.e. replace it with a '\0'
...
if (strlen(str) > strlen(longest_word))
    strcpy(longest_word, str)

EDIT: Also, you don't need the extra variable ch, you can just use str[a] in it's place. You should also define a constant for the file name, so you don't have to make multiple changes if you change the file name. #define FILENAME "words.dat" at the top, and replace every instance of "words.dat" in your program (lines 11 & 13) with FILE_NAME