Return to Answer

##Namespace std Stop doing this:

using namespace std;

See every other C++ code review and Why is “using namespace std;” considered bad practice?

##Monolithic Functions Break your code into multiple smaller functions. A good rule of thumb is no more than a dozen lines. If you give your function meaningful names then your code becomes self documenting.

##Global Variables Global variables are a bad idea. It makes testing hard. It makes debugging hard. You don't the result of the function biased on its inputs (it also depends on some other external global state that other functions may mutate while you are not watching.

Pass parameters to functions that should be the only state the worry about.

##Objects This looks like it should be an object:

map<string, food> foodMap_prot;
map<string, food> foodMap_carb;
map<string, food> foodMap_fat;

How about

class FoodMap
{
    typedef  std::map<std::string, Food>  FoodMap;

    FoodMap   protein;     // avoid the temptation to shorten names
    FoodMap   carbs;       // unless like this they are hard to spell.
    FoodMap   fat;
};

// Note on naming conventions.
// User defined types (usually) start with an Uppercase letters.
// Object usually start with a lowercase letter.
// This makes it easy to spot types over objects.

Looks like all the code in your main is designed to correctly fill up the map. So this should really be part of the FoodMap class. The class should be the only thing defining where things go internally. If you let functions (like main) fill up the map you have no control to validate input.

Namespace std

Stop doing this:

using namespace std;

See every other C++ code review and Why is “using namespace std;” considered bad practice?

Monolithic Functions

Break your code into multiple smaller functions. A good rule of thumb is no more than a dozen lines. If you give your function meaningful names then your code becomes self documenting.

Global Variables

Global variables are a bad idea. It makes testing hard. It makes debugging hard. You don't the result of the function biased on its inputs (it also depends on some other external global state that other functions may mutate while you are not watching.

Pass parameters to functions that should be the only state the worry about.

Objects

This looks like it should be an object:

map<string, food> foodMap_prot;
map<string, food> foodMap_carb;
map<string, food> foodMap_fat;

How about

class FoodMap
{
    typedef  std::map<std::string, Food>  FoodMap;

    FoodMap   protein;     // avoid the temptation to shorten names
    FoodMap   carbs;       // unless like this they are hard to spell.
    FoodMap   fat;
};

// Note on naming conventions.
// User defined types (usually) start with an Uppercase letters.
// Object usually start with a lowercase letter.
// This makes it easy to spot types over objects.

Looks like all the code in your main is designed to correctly fill up the map. So this should really be part of the FoodMap class. The class should be the only thing defining where things go internally. If you let functions (like main) fill up the map you have no control to validate input.

##Namespace std Stop doing this:

using namespace std;

See every other C++ code review and Why is “using namespace std;” considered bad practice?

##Monolithic Functions Break your code into multiple smaller functions. A good rule of thumb is no more than a dozen lines. If you give your function meaningful names then your code becomes self documenting.

##Global Variables Global variables are a bad idea. It makes testing hard. It makes debugging hard. You don't the result of the function biased on its inputs (it also depends on some other external global state that other functions may mutate while you are not watching.

Pass parameters to functions that should be the only state the worry about.

##Objects This looks like it should be an object:

map<string, food> foodMap_prot;
map<string, food> foodMap_carb;
map<string, food> foodMap_fat;

How about

class FoodMap
{
    typedef  std::map<std::string, Food>  FoodMap;

    FoodMap   protein;     // avoid the temptation to shorten names
    FoodMap   carbs;       // unless like this they are hard to spell.
    FoodMap   fat;
};

// Note on naming conventions.
// User defined types (usually) start with an Uppercase letters.
// Object usually start with a lowercase letter.
// This makes it easy to spot types over objects.

Looks like all the code in your main is designed to correctly fill up the map. So this should really be part of the FoodMap class. The class should be the only thing defining where things go internally. If you let functions (like main) fill up the map you have no control to validate input.

##Namespace std Stop doing this:

using namespace std;

See every other C++ code review and Why is “using namespace std;” considered bad practice?

##Monolithic Functions Break your code into multiple smaller functions. A good rule of thumb is no more than a dozen lines. If you give your function meaningful names then your code becomes self documenting.

##Global Variables Global variables are a bad idea. It makes testing hard. It makes debugging hard. You don't the result of the function biased on its inputs (it also depends on some other external global state that other functions may mutate while you are not watching.

Pass parameters to functions that should be the only state the worry about.

##Objects This looks like it should be an object:

map<string, food> foodMap_prot;
map<string, food> foodMap_carb;
map<string, food> foodMap_fat;

How about

class FoodMap
{
    typedef  std::map<std::string, Food>  FoodMap;

    FoodMap   protein;     // avoid the temptation to shorten names
    FoodMap   carbs;       // unless like this they are hard to spell.
    FoodMap   fat;
};

// Note on naming conventions.
// User defined types (usually) start with an Uppercase letters.
// Object usually start with a lowercase letter.
// This makes it easy to spot types over objects.

Looks like all the code in your main is designed to correctly fill up the map. So this should really be part of the FoodMap class. The class should be the only thing defining where things go internally. If you let functions (like main) fill up the map you have no control to validate input.

##Namespace std Stop doing this:

using namespace std;

See every other C++ code review and Why is “using namespace std;” considered bad practice?

##Monolithic Functions Break your code into multiple smaller functions. A good rule of thumb is no more than a dozen lines. If you give your function meaningful names then your code becomes self documenting.

##Global Variables Global variables are a bad idea. It makes testing hard. It makes debugging hard. You don't the result of the function biased on its inputs (it also depends on some other external global state that other functions may mutate while you are not watching.

Pass parameters to functions that should be the only state the worry about.

##Objects This looks like it should be an object:

map<string, food> foodMap_prot;
map<string, food> foodMap_carb;
map<string, food> foodMap_fat;

How about

class FoodMap
{
    typedef  std::map<std::string, Food>  FoodMap;

    FoodMap   protien;     // avoid the temptation to shorten names
    FoodMap   carbs;       // unless like this they are hard to spell.
    FoodMap   fat;
};

// Note on naming conventions.
// User defined types (usually) start with an Uppercase letters.
// Object usually start with a lowercase letter.
// This makes it easy to spot types over objects.

Looks like all the code in your main is designed to correctly fill up the map. So this should really be part of the FoodMap class. The class should be the only thing defining where things go internally. If you let functions (like main) fill up the map you have no control to validate input.

##Namespace std Stop doing this:

using namespace std;

See every other C++ code review and Why is “using namespace std;” considered bad practice?

##Monolithic Functions Break your code into multiple smaller functions. A good rule of thumb is no more than a dozen lines. If you give your function meaningful names then your code becomes self documenting.

##Global Variables Global variables are a bad idea. It makes testing hard. It makes debugging hard. You don't the result of the function biased on its inputs (it also depends on some other external global state that other functions may mutate while you are not watching.

Pass parameters to functions that should be the only state the worry about.

##Objects This looks like it should be an object:

map<string, food> foodMap_prot;
map<string, food> foodMap_carb;
map<string, food> foodMap_fat;

How about

class FoodMap
{
    typedef  std::map<std::string, Food>  FoodMap;

    FoodMap   protein;     // avoid the temptation to shorten names
    FoodMap   carbs;       // unless like this they are hard to spell.
    FoodMap   fat;
};

// Note on naming conventions.
// User defined types (usually) start with an Uppercase letters.
// Object usually start with a lowercase letter.
// This makes it easy to spot types over objects.

Looks like all the code in your main is designed to correctly fill up the map. So this should really be part of the FoodMap class. The class should be the only thing defining where things go internally. If you let functions (like main) fill up the map you have no control to validate input.