MyNewName
MyNewName

Reputation: 1055

C++ cout in method good programming style

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

Answers (4)

Denis Zaikin
Denis Zaikin

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

Emil Styrke
Emil Styrke

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

Sergei Tachenov
Sergei Tachenov

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

songyuanyao
songyuanyao

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

Related Questions