HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Parsing store sales content from a text file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filetextstoreparsingsalescontentfrom

Problem

I asked this question yesterday, and now I have come up with this code:

stock.h

#ifndef stock_stock_h
#define stock_stock_h
#include 

class  stock  {

public:
    stock() {
        itemName = " ";
        unitPrice = " ";
        quantityPurchased = " ";
        day = " ";
        month = " ";
        year = " ";
    }

    stock(std::string itemName,std::string unitPrice,std::string quantityPurchased,
          std::string day,std::string month,std::string year);

    std::string getItemName();
    std::string getUnitPrice();
    std::string getQuantityPurchased();
    std::string getDateDay();
    std::string getDateMonth();
    std::string getDateYear();

    void setItemDescription(std::string itemName);
    void setUnitPrice(std::string unitPrice);
    void setQuantityPurchased(std::string quantityPurchased);
    void setDateDay(std::string day);
    void setDateMonth(std::string month);
    void setDateYear(std::string year);

private:
    std::string itemName,unitPrice,quantityPurchased,day,month,year;
};
#endif


main.cpp

```
#include "stock.h"
#include
#include
#include
#include
#include
#include
#include
#include

using namespace std;

stock::stock(string itemName,string unitPrice,string quantityPurchased,string day,string month,string year) {

setItemDescription(itemName);
setUnitPrice(unitPrice);
setQuantityPurchased(quantityPurchased);
setDateDay(day);
setDateMonth(month);
setDateYear(year);

};

string stock::getItemName() {
return itemName;
}
string stock::getUnitPrice() {
return unitPrice;
}

string stock::getQuantityPurchased() {
return quantityPurchased;
}

string stock::getDateDay() {
return day;
}

string stock::getDateMonth() {
return month;
}

string stock::getDateYear() {
return year;
}

void stock::setItemDescription(std::string itemName) {
this->itemName = itemName;
}

void stock::setUnitPrice(std::string unitPrice) {
this->unitPrice = unitPrice;
}

void sto

Solution

-
stock doesn't need a default constructor if the starting values won't be useful. If you'll never need to create a stock object with default values, then you can just leave this out.

-
In private, declare each member once per line:

std::string itemName;
std::string unitPrice;
std::string quantityPurchased;
std::string day;
std::string month;
std::string year;


This should also be done in general as it helps maintain readability and maintainability.

-
Consider finding an alternative to all those accessors and mutators. They're not good for encapsulation as they can expose implementation details. It's best to have member functions that modify these data members within the class.

-
It seems that qualtityPurchased, day, month, and year should be of integer types, unless they specifically need to be strings.

-
It looks like you're including both stock's implementation and main() in main.cpp. As the file name suggests, only main() should be in this file. There should then be an additional .cpp file just for stock's implementation.

-
After you construct a stock object in main(), it's not quite clear what you're doing. Consider putting the contents of that for loop in a function with a descriptive name. The output after that could also be put into a separate display function or just stay in main().

Code Snippets

std::string itemName;
std::string unitPrice;
std::string quantityPurchased;
std::string day;
std::string month;
std::string year;

Context

StackExchange Code Review Q#38561, answer score: 6

Revisions (0)

No revisions yet.