-1

I hope that someone can help me since I'm clueless about the problem in my code. I am a new starter in programming and a week ago I tried an exercise which I completely failed. I tried to compare the elements in the array with the input text on the compiler. But the ("if") is always giving a false. Even if I write down an existing element of the array on the compiler. I know that there must be a lot of logical mistakes but I am really helpless. I am really looking for your answers. Thanks for the help!

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

struct category {
    char item_name[50];
    int item_quantity;
    float item_price;
};

void items_search (struct category a[], int length)
{
    char answer[] = { "yes" };
    int number = 0;
    int item_quantity = 0;
    char name[200];
    int i = 0;

    printf ("What are you looking for?\n");
    scanf ("%s", &name);

    if (a[i].item_name == name) {
        printf ("What is the desired number of the item?");
        scanf ("d", &number);
        fflush (stdin);
        if (number == a[i].item_quantity) {
            printf ("Order is possible. Should the order be placed?\n");
            scanf ("%s", &answer);
            fflush (stdin);
            if (answer == "yes") {
                puts ("The order is placed");
            }
            else {
                puts ("Order was cancelled");
            }
        }
        else {
            printf ("There are only %d pieces of the item. Should the "
                    "order be canceled?\n", a[i].item_quantity);
            scanf ("%s", &answer);
            fflush (stdin);
            if (answer == "yes") {
                puts ("Order was cancelled. Should be bought later?");
                scanf ("%s", &answer);
                fflush (stdin);
                if (answer == "yes") {
                    puts ("The order is placed");
                }
                else {
                    puts ("The order was cancelled");
                }
            }
            else {
                puts ("The order is placed");
            }
        }
    }
    else {
        printf ("The specified item does not exist\n");
    }
}

int main (void)
{
    struct category laptops[] = {
        {"Asus_365", 7, 499.00},
        {"Lenovo_L49", 30, 699.91},
        {"HP_Alien", 20, 649.99},
        {"Acer Alpha touch", 10, 899.99}
    };
    
    items_search (laptops, sizeof (laptops) / sizeof (struct category));
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
random
  • 1
  • 2
  • 1
    use strcmp function to test the equality (or not ) between 2 strings – nissim abehcera Sep 26 '21 at 17:06
  • Thanks for the answer. Unfortunately it does not work. It always say "specified order does not exist". – random Sep 26 '21 at 17:14
  • 2
    `scanf("%s",name);` (get rid of the `'&'`) `name` is already a pointer due to array-pointer conversion. You have the same problem with the other strings you attempt to read like `answer`. Further you must include a *field-width* modifier when using the `"%s"` conversion specifier or it is no safer than `gets()` See [Why gets() is so dangerous it should never be used!](https://stackoverflow.com/q/1694036/3422102) – David C. Rankin Sep 26 '21 at 17:18
  • Thanks a lot David C.Rankin. I will try it out. – random Sep 26 '21 at 17:27
  • Also note `fflush(stdin)` is undefined behavior according to the C-standard. It is only defined for "seekable" streams. There is only one compiler that allows `fflush(stdin)` to work as you are using it as a *Non-Standard* extension. – David C. Rankin Sep 26 '21 at 17:42
  • I also presume the formatting oddities of your code was from trying to format for this site. It is important to keep your code structured. If you are starting to code, then learn to Always compile with *warnings enabled*, and **do not** accept code until it *compiles without warning*. You can learn a lot from your compiler. Which one are you using? For gcc/clang use `-Wall -Wextra -pedantic -Wshadow` and add `-Werror` to have warnings treated as errors. Use `/W3` for VS. All other compilers have similar options. – David C. Rankin Sep 26 '21 at 17:49
  • Thank you David C. Rankin but I do not really understand why there is only one compiler that allows fflush(stdin) to work. Is there any alternative to fflush(stdin)? – random Sep 26 '21 at 17:51
  • Yes the formatting oddities are a result of trying to format the code for this site. I'm using gcc. How do I exactly add -Wall...? Is it enough to write it within the code? I am always hoping for compiler errors because then I can find a solution but those errors are mostly a syntax error. Unfortunately my code had logical mistakes. Thanks for the advice! – random Sep 26 '21 at 17:59
  • Does this answer your question? [How do I properly compare strings in C?](https://stackoverflow.com/questions/8004237/how-do-i-properly-compare-strings-in-c) – Adrian Mole Oct 04 '21 at 09:21

4 Answers4

2

You should not compare a pointer to a string:

if(answer=="yes"){

Instead, you should use strncmp:

if(strncmp( answer, "yes", 3) == 0)
user438383
  • 5,716
  • 8
  • 28
  • 43
Grzesl
  • 41
  • 5
  • Wow thank you alot Grzesl! It is not working perfectly but the "strncmp" changed a lot of things. Then should not I try it also in the beginnig with if (a[i].item_name == name) ? – random Sep 26 '21 at 17:20
  • Any time you are comparing strings in C, you should be using `strcmp`. Please see [the documentation for strcmp](https://en.cppreference.com/w/c/string/byte/strcmp) for details on its return value. _Unless you actually want to know if they point to the same memory address._ – Chris Sep 26 '21 at 17:43
  • All right understood. Is there a way to compare multiple strings with one string? I assume using switch could work. I guess to know whether both point the same memory address I have to compare the pointers? Thanks for the answers Chris! – random Sep 26 '21 at 18:06
1

For starters these calls of scanf

scanf ("%s", &name);

and

scanf ("%s", &answer);

are incorrect. You should write

scanf ("%s", name);

and

scanf ("%s", answer);

Arrays do not have the comparison operator. So in these if statements

 if (a[i].item_name == name) {

and

if (answer == "yes") {

there are compared two pointers to first elements of two different arrays (due to the implicit conversion of array designators to pointers to their first elements) that occupy different extents of memory. So their comparison will always evaluate to logical false.

Instead you need to use the C string function strcmp like

 if ( strcmp( a[i].item_name, name ) == 0 ) {

and

if ( strcmp( answer, "yes" ) == 0 ) {

Also calls of the function fflush with the stream stdin

fflush (stdin);

has undefined behavior. Remove such calls.

Pay attention to that the function declaration does not make a great sense relative to its definition because within the function there is used only the first element of the passed array.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • "Pay attention..." Exactly this was something that bothered me alot. It will alwas use the first element of the array. I tried to solve it with a loop but that also did not work. Perhaps I should use switch so that I can compare other elements of the array. Thank you for your advice and for you work. I'm really glad to receive so much help. – random Sep 26 '21 at 18:13
0

There are a large number of small problems in your code. The most critical are the attempts to use == for string-equality instead of strcmp(). The next being the inclusion of '&' before the array name when using scanf(). Upon access, an array is converted to a pointer to its first element, so there is no need for '&' before the array name in scanf() after "%s". See C11 Standard - 6.3.2.1 Other Operands - Lvalues, arrays, and function designators(p3)

Another issue is you must protect your array bounds when using the "%s" conversion specifier. It cannot be used safely otherwise. Why? "%s" will write as many non-whitespace characters as it is provided to the address given in the argument list. If given 500 bytes of shell-code, it will happily write all 500 bytes corrupting your stack everywhere "%s" is used with scanf(). (especially with answer which you declare as only 4-bytes long -- which is horribly insufficient) LESSON - Never skimp on buffer size!

To use the field-width modifier with scanf() when using "%s" you add the maximum number of bytes to read (not including the nul-terminating character), e.g. "199s" when writing to name. This ensures scanf() will not attempt to write more than 199 characters to name (plus the nul-terminating character)

All of this (and a lot more) are the pitfalls of scanf() and are why it is recommended to take all user input using fgets() or POSIX getline(). Provided a sufficiently sized buffer, the line-oriented input functions will consume an entire line of data. This eliminates all the problems inherent with scanf() and the only thing you need do is to trim the trailing '\n' read and included in the buffer they fill. This is trivial using strcspn(). It simply counts the number of initial characters that are not in its reject string -- providing an easy way to know how long the string is up to the '\n'. A better way to prompt, read/VALIDATE and trim the '\n' is:

    fputs ("What are you looking for?: ", stdout);  /* no conversion */
    /* read/VALIDATE all input with fgets.. 
     * it will consume a line at-a-time 
     * (given sufficient storage)
     */
    if (!fgets (name, MAXNM, stdin)) {
        return 0;
    }

    name[strcspn (name, "\n")] = 0;     /* trim '\n' from end of name */

The other issue you have is using fflush(stdin);. The way you are using it is Undefined according to the C standard. fflush is not defined for input streams, see: C11 Standard - 7.21.5.2 The fflush function(p2) and fflush(stdin); leads to undefined behaviour on most systems.. It is not portable. For example, on Linux, fflush is only valid on seekable streams. Which only applies to stdin if a file is redirected as input (e.g. ./prog <somefile). See: fflush(3) - Linux manual page. Full discussion Standard C and POSIX leave fflush(stdin) as undefined behaviour

(MS is the only compiler I know of that provides a Non-Standard implementation that does clear stdin the way you are using it. -- Don't do that, it is virtually 100% non-portable to any other compiler)

I won't go over all the points detailed in the other good answers. But one additional note as you are learning to program. Always compile with warnings enabled, and do not accept code until it compiles without warning. To enable warnings add -Wall -Wextra -pedantic to your gcc/clang compile string (also consider adding -Wshadow to warn on shadowed variables). You can add -Werror to have all warnings treates as errors (recommended) For VS (cl.exe on windows), use /W3. All other compilers will have similar options. Read and understand each warning -- then go fix it. The warnings will identify any problems, and the exact line on which they occur. You can learn a lot by listening to what your compiler is telling you.

To put things altogether for you what I've done is just to change your implementation to use fgets() instead of scanf() while leaving your code commented out below so you can easily see exactly what changes are necessary. I have commented the code heavily so you can understand why the changes were made. I also changed the return type from void to int so you can provide a meaningful return indicating whether your function succeeded or failed. You will see how that benefits your code in main().

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

#define MAXINM   50     /* if you need a constant, #define one (or more) */
#define MAXNM   200

struct category {
    char item_name[MAXINM];
    int item_quantity;
    float item_price;
};

/* choose a meaningful return type that can indicate whether
 * the function succeeded or failed. (0 - not found, 1 - found)
 */
int items_search (struct category a[], int length)
{
    char  answer[MAXNM] = "yes",    /* a string-literal initializer is fine */
          name[MAXNM] = "",
          tmp[MAXNM];               /* temporary array for input use */   
    int i = 0,                      /* group declarations by type */
        // item_quantity = 0,       /* UNUSED VARIABLE */
        number = 0,
        transaction = 0;            /* flag indicating success/failure */

    fputs ("What are you looking for?: ", stdout);  /* no conversion */
    /* read/VALIDATE all input with fgets.. 
     * it will consume a line at-a-time 
     * (given sufficient storage)
     */
    if (!fgets (name, MAXNM, stdin)) {
        return 0;
    }
    // scanf ("%s", &name);     /* scanf is full of pitfalls */
                                /* no & before name, it's a pointer */
    
    name[strcspn (name, "\n")] = 0;     /* trim '\n' from end of name */
    
    for (; i < length; i++) {   /* loop over each item in struct */
        if (strcmp (a[i].item_name, name) == 0) {   /* strcmp for equality */
            fputs ("Desired number of the item?: ", stdout);
            /* read/VALIDATE using fgets() */
            if (!fgets (tmp, MAXNM, stdin)) {
                return 0;
            }
            /* minimum validation of conversion with sscanf,
            * for full error reporting, use strtol()
            */
            if (sscanf (tmp, "%d", &number) != 1) {
                fputs ("error: invalid integer input.\n", stderr);
                return 0;
            }
            // scanf ("d", &number);
            // fflush (stdin);          /* defined for "seekable" streams */
            
            if (number <= a[i].item_quantity) { /* less than or equal */
                printf ("Order is possible.\n"
                        "Total Price: $%.2lf\n"
                        "Should the order be placed?: ",
                        number * a[i].item_price);
                /* read/VALIDATE using fgets() */
                if (!fgets (tmp, MAXNM, stdin)) {
                    return 0;
                }
                // scanf ("%s", &answer);
                // fflush (stdin);
                if (strcmp (answer, "yes") == 0) {
                    puts ("The order is placed");
                }
                else {
                    puts ("Order was cancelled");
                }
            }
            else {
                printf ("There are only %d pieces of the item.\nShould the "
                        "order be canceled?: ", a[i].item_quantity);
                
                /* read/VALIDATE using fgets() */
                if (!fgets (tmp, MAXNM, stdin)) {
                    return 0;
                }
                answer[strcspn (answer, "\n")] = 0;     /* trim '\n' */
                
                // scanf ("%s", &answer);   /* answer is already a pointer */
                // fflush (stdin);
                if (strcmp (answer, "yes") == 0) {   /* strcmp for equality */
                    puts ("Order was cancelled. Should be bought later?");
                    
                    /* read/VALIDATE using fgets() */
                    if (!fgets (tmp, MAXNM, stdin)) {
                        return 0;
                    }
                    answer[strcspn (answer, "\n")] = 0;     /* trim '\n' */
                    // scanf ("%s", &answer);
                    // fflush (stdin);
                    if (strcmp (answer, "yes") == 0) {      /* strcmp */
                        puts ("\nThe order for later is placed");
                    }
                    else {
                        puts ("\nThe order was cancelled");
                    }
                }
                else {
                    puts ("The order is placed");
                }
            }
            transaction = 1;        /* set transaction complete flag */
        }
        // else {   /* not needed here */
        //     printf ("The specified item does not exist\n");
        // }
    }
    
    return transaction;     /* return transaction status */
}

int main (void)
{
    struct category laptops[] = {
        { "Asus_365", 7, 499.00 },
        { "Lenovo_L49", 30, 699.91 },
        { "HP_Alien", 20, 649.99 },
        { "Acer Alpha touch", 10, 899.99 }
    };
    
    if (!items_search (laptops, sizeof laptops / sizeof *laptops)) {
        fputs ("error: incomplete transaction.\n", stderr);
    }
}

Example Use/Output

$ ./bin/transaction
What are you looking for?: HP_Alien
Desired number of the item?: 2
Order is possible.
Total Price: $1299.98
Should the order be placed?: yes
The order is placed

or

$ ./bin/transaction
What are you looking for?: HP_Gorilla
error: incomplete transaction.

Look things over and let me know if you have any further questions. NOTE: this isn't the only way to approach this, just one way that improves over the UNVALIDATED use of scanf() and eliminates the potential for exploit through buffer overflow from the unlimited use of "%s".

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • Thank you soo much:) Could you please tell me why "stdout" or "strcspn" must be used? – random Sep 27 '21 at 20:24
  • `fgets()` reads the `'\n'` generated by the user pressing `[Enter]` at the end of input and stores the `'\n'` as the last character in the buffer (character array) filled. You want to trim the `'\n'` from the end, because `strcmp ("yes", "yes\n")` don't compare equal. The declaration for `strcspn()` is `size_t strcspn(const char *s, const char *reject);` and it returns the initial number of characters is `s` that are not in the `reject` string. Using a `reject` of `"\n"` it tells you how many chars there are before the new line, e.g. `len = strcspn (s, "\n");` then `s[len] = '\0';` or just `0` – David C. Rankin Sep 28 '21 at 02:38
  • I just put it altogether as one `s[strcspn (s, "\n")] = 0;`. You use `stdout` with `fputs` to tell it which `FILE*` stream to write to. `stdout` is just standard output (like `stdin` is standard input and `stderr` is standard error). You use `fputs` when you need end-of-line control (you don't want a `'\n'` added like `puts()`). You choose `fputs()` over `printf()` when there are no conversions required (a good compiler will make the substitution for you -- but choosing the right function shows you understand which to use) – David C. Rankin Sep 28 '21 at 02:43
0

Where to start...I feel like its the 80's again

buffer overruns = BAD

scanf does no validation

there are other issues with your code I leave that as an exercise for you to figure out the differences.

here is a more functional version ...

    struct category {
     const char* item_name; // const is good
        int item_quantity; 
        double item_price; // float was too small
    };
    void items_search(category Category[])
    {
        char answer[4] = { "\0" }; // buffer was too small before
        int number = 0;
        int item_quantity = 0;
        char name[200] = { "\0" };
        int item = -1, Index = 0;;
        printf("What are you looking for?\n");
        scanf_s("%199s", &name, _countof(name)); // validation!
        bool loop = true;
        for (; nullptr != Category[Index].item_name && loop; Index++) {// does lower case comparison
           if (_stricmp(Category[Index].item_name, name) == 0) {
                item = Index;
                loop = false;
            }
        }
        if (-1 != item) { // did we find the item ?
            printf("What is the desired number of the item?");
            scanf_s("%d", &number);
            fflush(stdin);
            if (number <= Category[item].item_quantity) { // you only accepted exact answer
                printf("Order is possible. Should the order be placed?\n");
                scanf_s("%3s", &answer, _countof(answer));
                fflush(stdin);
                if (_stricmp(answer, "yes") == 0) {
                    puts("The order is placed");
                }
                else {
                    puts("Order was cancelled");
                }
            }
            else {
                printf("There are only %d pieces of the item. Should the order be canceled ? \n", Category[item].item_quantity);
                scanf_s("%3s", &answer, _countof(answer));
                fflush(stdin);
                if (_stricmp(answer, "yes") == 0) {
                    puts("Order was cancelled. Should be bought later?");
                    scanf_s("%3s", &answer, _countof(answer));
                    fflush(stdin);
                    if (_stricmp(answer, "yes") == 0) {
                        puts("The order is placed");
                    }
                    else {
                        puts("The order was cancelled");
                    }
                }
                else {
                    puts("The order is placed");
                }
            }
        } else {
            printf("The specified item does not exist\n");
        }
    }

the rest should be clear

   Also in main
    struct category laptops[] = {
        {"Asus_365", 7, 499.00},
        {"Lenovo_L49", 30, 699.91},
        {"HP_Alien", 20, 649.99},
        {"Acer Alpha touch", 10, 899.99},
        {nullptr,0,0 }
        };
        items_search(laptops); // see items_search above
  • Thank you very much. I already realised most of my mistakes but there is still a question left out for me. I've been taught that C has no bool values. How do you actually use it then? – random Sep 27 '21 at 20:27
  • if you have a strictly C compiler my code would need to be refactored slightly. bool and nullptr for example are c++ only I believe. You can always #define TRUE 1 and define FALSE or something similar. Or just use NULL for false and !NULL for true. a simple char t = 0; with 0 being false and anything else being true. Whatever works for you. Really there is no ONE way to do it theres lots of ways to do most anything. – Richard Day Sep 29 '21 at 04:23