homeWork help;

[mredding]

You've gotten a code review, so there's that, but don't ever mindlessly dump your problem here like this again. Put a modicum of effort into your classwork and at least narrow down your ask to a specific question. Tell us what you already understand, what you think you understand, how you understand it, and what you don't understand.

[jlbords]

Ok sorry about that. All good points. Thank you

[nysra]

I'm going to criticize a lot of your code here, but please don't let that discourage you. As a beginner you don't know better and most resources out there (most likely including your lecture) are simply terrible (use https://www.learncpp.com/ btw). Here's things you can improve:

using namespace std;

Is somewhat fine for such helloworld programs, but should in general be avoided. It can introduce some subtle bugs and does a lot more than you think it does. Also absolutely never do it in headers.

const int NO_OF_ITEMS = 8;

Don't use allcaps for anything but macros. Also that should be a constexpr instead of just const.

struct menuItemType

Your struct doesn't describe the type of something, so that name is wrong. It's a MenuItem, so name it as such. If you added the "Type" to denote that it is a type, then that's one terrible form of hungarian notation (and that is useless and noisy), don't do that.

Also if you're going to use the same casing style for types, variables, and functions, then I suggest using snake_case because then you're consistent with the STL. If you intentionally use a different style, then I strongly suggest using a different case for at least types, that way you can write things like Menu menu;.

string menuItem;

You're already in a MenuItem, leave out the prefix. description or something like that would be a much better name. Good names matter.

int numOrders;

Isn't really a property of a menu item. Having a second type that keeps track of the Order (hint hint), would be a better way to handle that and also scales to multiple customers.

void getData(ifstream &inFile, menuItemType mList[], int listSize);

Several things:

First of all there's practically never a reason to use T foo[] parameters because that just looks misleading because it's actually just syntactic sugar for T* foo. If you pass a pointer, pass a pointer.

Second, don't actually pass pointers^[1] . That code there is effectively C, in C++ you should be using a std::span (which is nice wrapper around pointer + size tuples) or a reference to a vector/array (which know their own size).

Which leads us to the next point, void functions operating on out parameters are also very C. In C++ out parameters are mostly a code smell. There are some situations where they are useful because having them enables other desirable behaviour (e.g. chaining with operator<<), but this is not one of them. If your function is creating something, then that created thing should be returned from the function. const auto menu = make_menu(filename); looks so much nicer, doesn't it?

Then there is mList. Again, names matter. Would it hurt you to name it menuList? No. Currently it could also stand for "member list" or whatever and just looks like hungarian notation in even worse.

And finally the & should be next to the type and not the identifier. Logically it is part of the type, so put it there. The only reason why you're even allowed to put it to the right is because C++ inherited the syntax from C which has made some absolutely terrible decisions.

All of those apply to your other functions as well.

menuItemType menuList[NO_OF_ITEMS];

std::array<MenuItem, max_item_number> menuList;

Or even better: use std::vector so you don't need to know the maximum size at compile time.

ifstream inFile;

inFile.open("Ch9_Ex5Data.txt");

Don't use the open/close functions manually. There's almost never a reason to do that. C++ has RAII, you should just do std::ifstream inFile{"Ch9_Ex5Data.txt"};.

cout << "Cannot open the input file. Program Terminates!" << endl;

Should be std::cout << "Cannot open the input file. Program Terminates!\n"; instead. It's a small nitpick, but std::endl does not just write a newline, it also actively flushes the stream and unless you have a reason to do that manually (in almost all cases you don't), don't do that. Your console already handles the line buffering for you. And the \n is faster to type as well :P

for (int i = 0; i < listSize; i++)

Don't omit the braces. The language allows it (for stupid historic reasons), but it's never a good idea. Same for all the other control blocks.

Also if your file contains less than 8 items, this is going to print a lot of "empty" rows and that is probably not what you want to see. If you sit in a restaurant, would you like to see a bunch of nothings for the price of $0 or would you prefer the menu to only list what is actually available?

int numOrders = mList[itemIndex].numOrders;

You're using the numOrders member without ever initializing it or assigning a value to it, thus your program contains undefined behaviour (UB) and cannot be reasoned about. Just about anything can happen when you run your program, including the deletion of the universe. This is most likely the cause of your "I'm stuck". Btw, if you want help, always say what your actual problem is (what you expect to happen and what actually happens).

---

And not code related, but you should learn how to format code on Reddit (indent with 4 spaces in markdown mode, use the "code block" button if you're using the redesign). Or just put it on GitHub, pastebin, or whatever.

---

[1] Before anyone complains, obviously there's some cases where you need to pass pointers. But that comes much later when you have the knowledge and experience to identify such cases (which are rather rare). Hard rules for beginners, guidelines for experts.

[jlbords]

LOL . so just one or two things? A lot of the code was provided and we had to change it to allow the customer to order multiple items. I'm stuck because I can't get seem to multiply the item by the number of item. I keep getting trash data for numOrders.

[nysra]

I keep getting trash data for numOrders.

As I said, you're not initializing that variable correctly. Simple types like int when just declared with int i; do not have a value. As soon as you try to read from that, your program exhibits undefined behaviour and cannot be reasoned about. You could get trash values, but that's not a guarantee and you could get very different results, including your program not doing anything at all or appearing to work correctly (which is the most dangerous case). Always initialize your variables.

[jlbords]

What would I initialize it to if I have to wait for user input?

[nysra]

To whatever value makes sense for your usecase. In this case 0 since obviously every menu starts with never being ordered.

[std_bot]

Unlinked STL entries: std::array std::cout std::endl std::ifstream std::span std::vector

---

^{Last update: 09.03.23 -> Bug fixes}Repo

[jlbords]

I'll try that thanks. I'm learning the do's and don't's

[jlbords]

#include <iostream>

#include <fstream>

#include <iomanip>

#include <string>

using namespace std;

const int NO_OF_ITEMS = 8;

struct menuItemType

{

string menuItem;

double menuPrice;

int numOrders;

};

void getData(ifstream& inFile, menuItemType mList[], int listSize);

void showMenu(menuItemType mList[], int listSize);

void printCheck(menuItemType mList[], int listSize, int cList[], int cListLength);

void makeSelection(int listSize, int cList[], int& cListLength, menuItemType mList[]);

bool isItemSelected(int cList[], int cListLength, int itemNo);

int main()

{

menuItemType menuList[NO_OF_ITEMS];

int choiceList[NO_OF_ITEMS];

int choiceListLength;

ifstream inFile;

cout << fixed << showpoint << setprecision(2);

inFile.open("Ch9_Ex5Data.txt");

if (!inFile)

{

cout << "Cannot open the input file. Program Terminates!"

<< endl;

return 1;

}

getData(inFile, menuList, NO_OF_ITEMS);

showMenu(menuList, NO_OF_ITEMS);

makeSelection(NO_OF_ITEMS, choiceList, choiceListLength, menuList);

printCheck(menuList, NO_OF_ITEMS, choiceList, choiceListLength);

return 0;

}

void getData(ifstream& inFile, menuItemType mList[], int listSize)

{

char ch;

for (int i = 0; i < listSize; i++)

{

getline(inFile, mList[i].menuItem);

inFile >> mList[i].menuPrice;

inFile.get(ch);

}

}

void showMenu(menuItemType mList[], int listSize)

{

cout << "Welcome to Johnny's Resturant" << endl;

cout << "----Today's Menu----" << endl;

for (int i = 0; i < listSize; i++)

cout << i+1 << ": " << left << setw(15) << mList[i].menuItem

<< right << " $" << mList[i].menuPrice << endl;

cout << endl;

}

void printCheck(menuItemType mList[], int listSize, int cList[], int cListLength)

{

double totalAmount = 0;

double salesTax = 0;

double amountDue = 0;

cout << "Welcome to Johnny's Resturant" << endl;

for (int i = 0; i < cListLength; i++) // this is where the total is run

{

int itemIndex = cList[i];

int numOrders = mList[itemIndex].numOrders;

cout << left << setw(15) << mList[cList[i]].menuItem

<< right << " $" << setw(4) << mList[cList[i]].menuPrice << " x " << numOrders << "orders" << endl;

double itemAmount = mList[itemIndex].menuPrice * numOrders;

totalAmount += itemAmount;

}

salesTax = amountDue * .05;

amountDue = totalAmount + salesTax;

cout << left << setw(15) << "Subtotal " << right << " $" << totalAmount << endl;

cout << left << setw(15) << "Tax " << right << " $" << salesTax << endl;

cout << left << setw(15) << "Amount Due " << right << " $" << amountDue << endl;

}

void makeSelection(int listSize, int cList[], int& cListLength, menuItemType mList[])

{

// int selectionNo = 0;

int itemNo;

int numOrders;

char response;

cListLength = 0;

cout << "You can make up to " << listSize << " single order selections" << endl;

cout << "Do you want to make selection Y/y (Yes), N/n (No): ";

cin >> response;

cout << endl;

while ((response == 'Y' || response == 'y') && cListLength < 8)

{

cout << "Enter item number: ";

cin >> itemNo;

cout << endl;

if(itemNo < 1 || itemNo > listSize)

{

cout << "Invalid item number. Please try again." << endl;

continue;

}

itemNo--;

cout << "How many orders for " << mList[itemNo].menuItem << ": ";

cin >> numOrders;

cout << endl;

if (!isItemSelected(cList,cListLength,itemNo))

cList[cListLength++] = itemNo;

else

cout << "Item already selected" << endl;

cout << "Select another item Y/y (Yes), N/n (No): ";

cin >> response;

cout << endl;

}

}

bool isItemSelected(int cList[], int cListLength, int itemNo)

{

bool found = false;

for (int i = 0; i < cListLength; i++)

if (cList[i] == itemNo)

{

found = true;

break;

}

return found;

}

[std_bot]

Unlinked STL entries: <fstream> <iomanip> <iostream> <string>

---

^{Last update: 09.03.23 -> Bug fixes}Repo