Reputation: 1055
What do you thing, should I use cout <<
in methods? Or should I return an array? Because I need more then one return value
.
I have the class SGetraenkeAutomat
, here is the .h
file
class SGetraenkeAutomat
{
public:
// this method
void DisplayInventory();
SGetraenkeAutomat();
SGetraenkeAutomat(int nColaAnzahl, int nSpriteAnzahl, int nFantaAnzahl);
virtual ~SGetraenkeAutomat();
private:
int m_nColaAnzahl;
int m_nSpriteAnzhal;
int m_nFantaAnzahl;
};
the method void SGetraenkeAutomat::DisplayInventory()
in SGetraenkeAutomat.cpp
void SGetraenkeAutomat::DisplayInventory(){
std::cout << m_nColaAnzahl;
std::cout << m_nSpriteAnzahl;
std::cout << m_nFantaAnzahl;
}
Is that a good programming style?
-> I dont think so, but maybe someone of you can explain it me.
Upvotes: 1
Views: 299
Reputation: 579
The answer depends on your task. If you need to display variables than use some stream to display. If other classes from your program require data from your SGetraenkeAutomat class than you should provide public getters for each private member. For ex:
int get_ColaAnzahl()
{
return m_nColaAnzahl;
}
As I can see in your case will be better to create some base class (lets name it Drink):
struct Drink {
int count;
std::string name;
}
Then modify your SGetraenkeAutomat class to:
class SGetraenkeAutomat
{
public:
// this method
void DisplayInventory() {
for (auto drink: drinks) {
std::cout << drink.name << ": " << drink.count << std::endl;
}
}
void addDrink(const Drink& drink) {
drinks.push_back(drink);
};
SGetraenkeAutomat();
virtual ~SGetraenkeAutomat();
private:
std::vector<Drink> drinks;
};
I think you understand my idea described above.
Upvotes: 1
Reputation: 1071
In this case, because the method is called DisplayInventory
, one would not expect it to return anything, but actually show the inventory to the user in some way. This is in alignment with the Tell, Don't Ask "rule" which says you should tell your objects what to do, not ask them questions about their state.
In a simple application, using cout
to interact with the user is perfectly acceptable; another option would be to use a GUI to display the information.
You could also look at it this way: to display the inventory to the standard output, you must use cout
somewhere. It is certainly better (in my opinion) to encapsulate it in a class method than exposing all the data with a getter (thus breaking "tell, don't ask") and printing it outside the class.
If you want to get more fancy and get the output customizable, you could create some kind of formatter class and pass it to the display function (Sergey's answer is a variant of this principle).
Upvotes: 1
Reputation: 24879
I'd rather rename it to PrintInventory
and provided an optional argument:
void PrintInventory(std::ostream &output = std::cout);
This way you can output your inventory to some other stream and at the same time you don't have to specify it each time you want to print to stdout.
void SGetraenkeAutomat::PrintInventory(std::ostream &output){
output << m_nColaAnzahl;
output << m_nSpriteAnzahl;
output << m_nFantaAnzahl;
}
This way you can do either:
automat.PrintInventory(); // prints to stdout
or something like
automat.PrintInventory(std::cerr); // prints to stderr, for example
Upvotes: 2
Reputation: 172934
Or should I return an array?
No. The class has 3 private
members, it's better not to expose them outside, which result in tight coupling at most case.
should I use cout << in methods?
If it statisfies the request, it should be fine.
Upvotes: 1