How can I improve these functions?

Discussion in 'C Programming' started by S Kemplay, Apr 4, 2004.

  1. S Kemplay

    S Kemplay Guest

    Hi all,

    I am after any pointers you may have to improve these two functions.

    They both work but are part of an assignment - I am looking for feedback.

    The first one reads records from a file - the teacher won't allow a
    delimeted file he wants spaces between words joined with an underscore.

    The records are read into an array of structs:

    typedef struct drink
    {
    int stockCode;                          
        char drinkName[45];                     
        char manufacturer[25];          
        float purchasePrice;
    } DRINK;

    void option1(DRINK drinkArray[], DRINK drinkRec)
    {
        FILE* fp = fopen("products.txt", "r");
        char line[255] = "";
        int count = 0;
        printf("\nRead records from file\n");
        printf("======================\n\n");
        if(fp)
        {
            while((fgets(line, sizeof(line), fp) != NULL))
            {
                sscanf(line, "%d %44s %24s %f\n", &drinkRec.stockCode,
                drinkRec.drinkName,  drinkRec.manufacturer,
                &drinkRec.purchasePrice);
                if(addDrinkArray( drinkArray, drinkRec))
                    ++count;
                else
                {
                    printf("Array is full: ");
                    break;
                }
            }
        fclose(fp);
        }
        else
            printf("Error opening file");

        printf("%d records succsefffully added\n", count);
    }

    The second function is to validate user input - I need a float for
    purchasePrice. I have a similar one for an int.

    I want to be able to accept a newline as valid when this function is used to
    get input to update a record ie "Enter a purchase price or press enter to
    leave unchanged" so I have set a flag. If the flag is true, the return
    signals whether a newline was entered or not.

    int getValidPurchasePrice(float* purchasePrice, int bypass)
    {
        char line[255];
        char* test;
        int retval = 1;
        while(1)
        {
            printf("Please enter the purchase price ");
            fgets(line, sizeof(line), stdin);
            *purchasePrice = strtod(line, &test);
            if(isspace(*test) && line[0] != '\n')
            {
                 break;
            }
            else if(line[0] == '\n' && bypass != 0)
            {
                retval = 0;
                break;
            }
            line[strlen(line) - 1] = '\0';
            printf("%s is not a valid purchase price\n", line);
        }
        return retval;
    }


    Thanks in advance

    Sean Kemplay
     
    S Kemplay, Apr 4, 2004
    #1
    1. Advertisements

  2. S Kemplay

    Malcolm Guest

    /*
    First I'll give you local improvements, then I'll explain how to redesign.
    */
    /*
    You want to check that a full line was read by looking for the newline.
    */
    sscanf() returns the number of items successfully converted. Check the
    return value and bale if the record is malformed.
    /*
    This is a bit lame. You need to be able to read records until the computer
    runs out of memory.
    */
    /*
    You might consider printing errors to stderr.
    */
    Why not rewrite the function

    /*
    Returns an allocated array of N drinks read from the file.
    */
    DRINK *readdrinks(char *fname, int *N)
    {
    }
    Also, you could get rid of all the output to printf() and put it somewhere
    else, so the function is more portable.
    Write the function

    int validmoney(const char *str, double minprice, double maxprice)
    {
    Call strtod. Check for garbage following what you read. This tells
    you you have a valid floating-point number.
    Checking for min price and max price is trivial (min might be 1 cent, max
    can be DBL_MAX or you might want to sanity check).
    However 12 and 12.34 are valid, 12.3 and 12.345 are not. You need to use
    strchr() to seach for the decimal point and, if it exists, count the digits
    (using isdigit). If there are two then you are OK, if not then reject it.
    }

    Input can also be invlaid if fgets() fails to read a line. keep this
    separate from your validmoney() function.
     
    Malcolm, Apr 4, 2004
    #2
    1. Advertisements

  3. S Kemplay

    S Kemplay Guest

    Thankyou for your suggestions Malcolm, as well as being more failsafe, my
    functions are a lot more portable now.

    Cheers

    Sean
     
    S Kemplay, Apr 5, 2004
    #3
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.