1

I am having a situation with strncmp function in C, it is returning 0 even when the words do not match, in the example below, I am testing it with the letter 'R' and when running the code it returns 0 even when the compared word in the txt document is 'RUN'. Do you happen to know whether

Am I missing something in the strncmp function or somewhere else in my code?

Thank you for your input.


bool lookup(string s);

int main(void) {

char *s;
s = "R";
if (lookup(s)) {
    printf("Word found =)\n");
} else {
    printf("Word not found =(\n");
}
}

// Looks up word, s, in txt document.
bool lookup(string s)
{
 // TODO
    char *wordtosearch;
    wordtosearch = s;
    int lenwordtosearch = strlen(wordtosearch);
    char arraywordindic[50];

// Open txt file
FILE *file = fopen("text.txt", "r");
if (file == NULL)
{
    printf("Cannot open file, please try again...\n");
    return false;
}

while (!feof(file)) {
    if (fgets(arraywordindic, 50, file) != NULL) {
        char *wordindic;
        wordindic = arraywordindic;
        int result = strncmp(wordindic, wordtosearch, lenwordtosearch);
        if (result == 0) {
            printf("%i\n", result);
            printf("%s\n", wordindic);
            printf("%s\n", wordtosearch);
            fclose(file);
            return true;
        }
    }        
}
fclose(file);
return false;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • 1
    `strncmp` compares only first `lenwordtosearch` (the third argument) characters and returns 0 iff they matches, so your code seems working well. What is your expected behavior? – MikeCAT Aug 22 '20 at 03:17
  • By the way, C don't have standard `string` type. Are you using `cs50.h`? – MikeCAT Aug 22 '20 at 03:19
  • Hi MikeCAT, yes cs50.h. The thing is that it compares R with RUN and it gives 0. I want it to return 0 when it finds R only. – Logan Cortés Aug 22 '20 at 03:33
  • read [why while( feof() ) is always wrong](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) – user3629249 Aug 23 '20 at 00:02
  • regarding: `while (!feof(file)) { if (fgets(arraywordindic, 50, file) != NULL) {` suggest combining into a single statement: `while ( fgets(arraywordindic, 50, file) ) {` – user3629249 Aug 23 '20 at 00:04
  • OT: the posted code contains some 'magic' numbers. 'magic' numbers are numbers with no basis. I.E. 50. 'magic' numbers make the code much more difficult to understand, debug, etc. Suggest: use a `enum` statement or `#define` statement to give that 'magic' number a meaningful name. Then use the meaningful name throughout the code. – user3629249 Aug 23 '20 at 00:08
  • OT: regarding: `printf("Cannot open file, please try again...\n");` This is just telling the user that some error occurred, not why it occurred. Also error messages should be output to `stderr` and when the error is from a C library function, should also output the text reason the system, thinks the error occurred. Suggest: `perror( "fopen failed" );` as that will output both the error message and the text reason to `stderr`. – user3629249 Aug 23 '20 at 00:12
  • regarding: `if (fgets(arraywordindic, 50, file) != NULL) {` This will also input the trailing newline `'\n'` which will ruin any string comparison. Suggest the next statement be: `arraywordindic[ strcspn( arraywordindic, "\n" ) ] = '\0';` – user3629249 Aug 23 '20 at 00:17
  • regarding; `int lenwordtosearch = strlen(wordtosearch);` The function: `strlen()` returns a type `size_t` which is a `unsigned long int` The result is trying to push a unsigned value into a signed value. This should be avoided – user3629249 Aug 23 '20 at 00:23
  • this function: `int strncmp(const char *s1, const char *s2, size_t n);` expects the third parameter to have type `size_t`, so `lenwordtosearch` needs to have type `size_t`, not `int` – user3629249 Aug 23 '20 at 00:25
  • Thank you all for your valuable input. Will make adjustments as needed. – Logan Cortés Aug 24 '20 at 02:30

2 Answers2

1
int result = strncmp(wordindic, wordtosearch, lenwordtosearch);

This is going to give you zero if the first lenwordtosearch characters of wordtosearch matches the first lenwordtosearch characters of any word in the dictionary.

Given that the word you're searching for is S, any word in the dictioanary that starts with S is going to give you a match.

You should probably be checking the entire word. That probably means cleaning up the word you've read in from the file (i.e., removing newline) and using strcmp() instead, something like:

wordindic = arraywordindic;

// Add this:
size_t sz = strlen(wordindic);
if (sz > 0 && wordindic[sz - 1] == '\n')
    wordindic[sz - 1] = '\0';

// Modify this:
// int result = strncmp(wordindic, wordtosearch, lenwordtosearch);
int result = strcmp(wordindic, wordtosearch);
paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
  • Hello paxdiablo, thank you, I edited the post as I made a typo. The example test letter I am using is R and let´s say the word in the text document is RUN, R and RUN are not the same, hence strncmp should give 0, unless there is an R only in the text document. At least that is how I understand strncmp works. As per tutorialspoint.com: "The C library function int strncmp(const char *str1, const char *str2, size_t n) compares at most the first n bytes of str1 and str2." – Logan Cortés Aug 22 '20 at 03:44
  • @Logan, if you're using the length of `"R"` to compare the two strings (and you are), you'll get a match on every word starting with `R`. `R` and `RUN` may be different but you're comparing `R` against the ***first letter*** of `RUN`. So, yes, your understanding of `strncmp()` is wrong :-) – paxdiablo Aug 22 '20 at 03:53
  • A useful idea: When overwriting the `'\n'`, decrement the `sz`. Then it remains a useful length for later code. `wordindic[sz - 1] = '\0';` --> `wordindic[--sz] = '\0';` – chux - Reinstate Monica Aug 22 '20 at 06:08
  • Thank you @paxdiablo – Logan Cortés Aug 24 '20 at 02:40
1

The thing is that it compares R with RUN and it gives 0. I want it to return 0 when it finds R only.

In this case you need to compare whole words using the function strcmp instead of comparing only lenwordtosearch characters using the function strncmp.

Take into account that the function fgets can append the new line character '\n' to the entered string. You need to remove it before comparing strings.

if (fgets(arraywordindic, 50, file) != NULL) {
    arraywordindic[ strcspn( arraywordindic, "\n" ) ] = '\0';
    int result = strcmp(arraywordindic, wordtosearch);
    if (result == 0) {
        printf("%i\n", result);
        printf("%s\n", arraywordindic);
        printf("%s\n", wordtosearch);

As a result these declarations

int lenwordtosearch = strlen(wordtosearch);

and

char *wordindic;
wordindic = arraywordindic

may be removed.

And the condition of the while loop should be written like

while ( fgets(arraywordindic, 50, file) != NULL ) {
    arraywordindic[ strcspn( arraywordindic, "\n" ) ] = '\0';
    int result = strcmp(arraywordindic, wordtosearch);
    if (result == 0) {
        printf("%i\n", result);
        printf("%s\n", arraywordindic);
        printf("%s\n", wordtosearch);
    //...    
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335