Creating an inventory using classes

The next assignment in my class is to create a program that allows the user to input an inventory, and then display the total value of the inventory. We are required to create a class with both private and public member functions. I tried to keep it as streamlined as possible but I know there is always room for improvement. Let me know what you think.

I defined the member functions outside of the class by direction of the prof. I personally like seeing it all in one place, but this is an OOP class, so I guess it makes sense to define them outside. The only other direction was that all values have to be positive, hence the do while loops.

```
#include

using namespace std;

class inventory
{
private:
int productNum[10];
int productCount[10];
double productPrice[10];

public:
int locator;
void retrieve();
void init();
void store();

}inv1;

void setItemNumber(int &temp)
{
do
{
cout > temp;
} while (temp > temp;
} while (temp > price;
} while (price < 0);
}

void inventory :: init()
{
int counter = 0;
while (counter < 10)
{
productCount[counter] = 0;
productNum[counter] = 0;
productPrice[counter]= 0;
counter++;
locator = counter;
}
}

void inventory :: store()
{
int counter = 0, temp = 0;
double price = 0;
cout << "\nWelcome to the inventory control system" << endl;
cout << "When finished please enter '0' for the item number" << endl;
while (counter < locator)
{
setItemNumber(temp);
if(temp == 0)
{
locator = counter;
return;
}
productNum[counter]=temp;
setQuantity(temp);
productCount[counter]=temp;
setPrice(price);
productPrice[counter]=price;
counter++;
}
}

void inventory :: retrieve()
{
int counter = 0;
double total = 0;
cout << "Product Number \t\

int productNum[10];
int productCount[10];
double productPrice[10];

That's not exactly OOP what you did here. A single product is apparently defined by the combination of productNum, productCount and productPrice. So these 3 attributes should have been grouped together in a product class:

class product {
    int productNum;
    int productCount;
    double productPrice;
}

I'll let it to your exercise to decide on the visibility of these attributes, and to add a constructor.

And the inventory class then correspondingly holds a collection of products instead:

product products[10];

Well, except that this isn't exactly a clean way to manage a collection of products in C++ either, better use a dynamically growing data type with checked bounds:

std::vector products;

This also brings us to the next point, your use of these two:

int locator;
int counter = 0;

I assume your locator variable was actually supposed to indicate the fill level of the original inventory tracking mechanism. Please try to name it properly the next time. There is also a second problem with locator, you made it public even though any external modification to this variable would break the integrity of your implementation.

void inventory::init() {
    ...
}

This method should have been called void inventory::inventory(), which makes it the constructor for the inventory class and is called automatically every time a new inventory is instantiated. Have a read on the RAII principle.

void inventory::store();
void inventory::retrieve();

These two have a rather strange signature for what they are called. Your task was to create an inventory class which manages an inventory. No less, no more. What you actually did, was cramming all the input and output logic into that class as well. That's a direct violation of the Separation of Concerns principle.

What that part of the API should look like is simple:

void inventory::store(product p);
double inventory::getTotal();

These methods shouldn't contain any input/output logic, but only the algorithms relevant to storing products, respectively to query all stored products for the grand total.

The input/output belongs into the scope working with the inventory instance, not into that class.

class inventory {...} inv1;

That's a global variable you created here. Unless you have a very good reason to do so (and trust me when I say you almost never have), don't. You should have instantiated inventory inside the main() function instead.


I defined the member functions outside of the class by direction of the prof. I personally like seeing it all in one place, but this is an OOP class, so I guess it makes sense to define them outside.

Not quite. You should probably be familiar by now with the concept of splitting your source code into header and source files. With headers containing structural information, and source files containing implementations.

In this case, they class definition should have gone into a file called inventory.h and the implementation of the class would have belonged into inventory.cpp.

If take a look back at the section about the global variable inv1, it should strike why that was a mistake.


We are required to create a class with both private and public member functions.

You don't have any private methods yet. Just food for thought, in regard to the Separation of Concerns, you could actually structure your API like this:

class inventory {
    private:
        // Actually stores the product
        void storeUnchecked(product p);
        ...
    public:
        // Validates that the product fulfills the requirements
        // If yes, passes down to storeUnchecked()
        bool store(product p);
        ...
}

Since adding a product can obviously fail if validation fails, you want to account for that by allowing your API to signal the success or failure back to the calling location.

StackExchange Code Review Q#142394, answer score: 8