2
\$\begingroup\$

I have just started a course on C and wanted to test what I have learned so far. I did so by making a small program that tells you how much you should charge for a shot of a spirit you put in.

The code will ask if you want to do a calculation. It will follow up with a question about the name, price and size of a product and respond with the calculation.

I have just learned how to use a Struct and just worked on Arrays. My next step will be to try and implement arrays to store the Structs you made and have the option to return to previous input. How did I do so far and what could I improve on?

#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <math.h>
#include <ctype.h>

//structs
typedef struct
{
    string name;
    float price;
    int size;
} Bottles;


//prototypes
Bottles get_newBottle(void);

//main
int main(void)
{
    string newBottles;
    //Ask user for input, repeat untill answered correctly.
   do
   {
       do
        {
            newBottles = get_string("Do you want to calculate a new shot y/n? ");
            if (strcmp(newBottles, "y") == 0)
            {
                //Build struct
                Bottles newBottle = get_newBottle();
                int shotSize = get_int("What is the size of your pour of %s in ML? ", newBottle.name);
                int bevCost = get_int("What percentage beverage cost do you want your drink to be? ");
                float shotCost = (newBottle.price / newBottle.size) * shotSize;
                float bevPrice = ((shotCost / bevCost) * 100) *1.21;
                printf("The cost of your shot of %s is %.2f Euro. \n", newBottle.name, shotCost);
                printf("At the cost of %.2f you will have to charge %.2f Euro including 21 percent VAT. \n", shotCost, bevPrice);

            }
            else if(strcmp(newBottles, "n") == 0)
            {
                printf("ok. doei!\n");
            }
        }
        while (! (strcmp(newBottles, "y") == 0) && ! (strcmp(newBottles, "n") == 0));
    }
    while (strcmp(newBottles, "y") == 0);
    return 0;
}



//extra functions
Bottles get_newBottle(void)
{
    string name = get_string("What is the name of your product? ");
    float price = get_float("What is the price of your product? ");
    int size = get_int("What is the size in ML of your product? ");

    Bottles newName = {name, price, size};
    return newName;
}
\$\endgroup\$
5
  • \$\begingroup\$ Is this C or C++? std::string is a C++ class. \$\endgroup\$ – Reinderien Jan 15 at 16:14
  • \$\begingroup\$ And where is get_string defined? Probably in cs50.h; please show its contents. \$\endgroup\$ – Reinderien Jan 15 at 16:15
  • 2
    \$\begingroup\$ I think string is the awful CS50 typedef of char *. \$\endgroup\$ – Toby Speight Jan 15 at 16:32
  • \$\begingroup\$ I used the IDE from harvards CS50. They added the get_string. You can see it's contents here: manual.cs50.io \$\endgroup\$ – Thomas Doll-Datema Jan 15 at 17:19
  • \$\begingroup\$ So, no, I can't; that's the manual and not the source. The source is here. \$\endgroup\$ – Reinderien Jan 15 at 18:51
3
\$\begingroup\$

"Utility" libraries

It's time to take the training wheels off, so to speak. You need to stop using cs50.h and replace it with standard calls to the C libraries. The implementation for get_string has a careful, dynamically-allocated buffer algorithm that is really not necessary for most purposes and can be replaced with simpler calls that use a fixed-size buffer. Advantages of a fixed-size buffer, other than extremely reduced complexity, include that it can enforce "domain-specific limits" (i.e. your product name cannot exceed 128 characters, or whatever) where get_string cannot. get_string is especially overkill for your yes/no prompt.

For an overrun-safe, simple method that is a fixed-buffer replacement for get_string, consider fgets.

Spelling

untill -> until

Memory leaks

You call get_string in a loop, which allocates a new buffer every time; and you never free it. So this will happily eat all of your RAM if you let it.

Comparisons

! (strcmp(newBottles, "y") == 0)

should just be

strcmp(newBottles, "y") != 0

New structure initialization

It's somewhat unusual to be returning a structure instance from get_newBottle, though it isn't the end of the world. More typical is to see either

  1. initialization of an existing structure passed by pointer, or
  2. allocation and initialization of a new structure via malloc then returning a pointer.

This impacts performance more for huge structures, but it's still a good idea to learn.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for taking the time to write all this out! I have changed the Comparison and the spelling error. How would I bets go about removing the get_string from the loop? This weeks subject of the course is memmory and working with malloc so I'll get back on changing that bit of code! \$\endgroup\$ – Thomas Doll-Datema Jan 16 at 13:32
  • 1
    \$\begingroup\$ Read about fgets; edited. \$\endgroup\$ – Reinderien Jan 16 at 17:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.