how to cout a subclass after static cast into a vector

Question

I have two classes, Article and a subclass ArticleOnSale (public inheritance).

And also a class Boutique, which contains a vector<Article*> arts.

I have defined the << operator for both Article and ArticleOnSale

In Articles.cpp I have a public Addarticle

void Addarticle(Article* a)
{
    Article* p;
    if (typeid(*a) == typeid(Article))
    {
        p = new Article(a);
        arts.push_back(p);
        cout << "\narticle\n";
    }
    else
    {
        p = new ArticleEnSolde(static_cast<const ArticleEnSolde&>(*arts[arts.size()])); //creates a huge problem 
        arts.push_back(p);
    }
}

When I want to print out the articles, it only uses the << of Article for both Article and ArticleOnSale.

ostream & operator<<(ostream& n, Boutique a)
{
    n<<"\nshowing boutique:\n";
    Article* p;
    for (int i = 0; i < a.arts.size(); i++)
    {
        p = a.arts[i];
        if (typeid(*a.arts[i]) == typeid(Article))
        { 
            n << *a.arts[i];
            n << "\narticle not on sale detected\n";
        }
        else 
        {
            n << "i can detect article on sale";//just debugging (doesn't work)
        }
    }
    return n;
}

You are missing some fundamental concepts of how C++ works. Your `a` parameter will ***always*** be an `Article`, so comparing to a `typeid` of an `Article` will ***always*** be true. Furthermore, there are very few reasons in C++ to resort to using `typeid` in the first place. There's rarely a valid reason to do so. Do you [have a good C++ textbook](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) where you can learn more about how to correctly pass parameters to functions, and using inheritance? — Sam Varshavchik, Apr 14 '21 at 23:46

mattlangford · Accepted Answer · 2021-04-14T23:58:48.667

A possible implementation that takes better advantage of virtual functions would be something like this:

class Article {
public:
    virtual bool is_on_sale() const { return false; }
};

class ArticleOnSale {
public:
    virtual bool is_on_sale() const { return true; }
};

then your vector would need to look like:

std::vector<std::unique_ptr<Article>> arts;

// add an Article:
arts.push_back(std::make_unique<Article>());

// or on sale
arts.push_back(std::make_unique<ArticleOnSale>());

(the unique_ptr is needed so that the C++ virtual functions will behave, without the pointer-ness of it this wouldn't work).

Finally, the ostream operator becomes fairly concise:

ostream & operator<<(ostream& os,Boutique a ) {
    for (auto& ptr : a.arts) {
        os << (ptr->is_on_sale() ? "article on sale " : "article is full price ")
    }
}

Of course there may be better ways of going about, like maybe just storing a boolean variable inside the Article if it's on sale - but I hope that makes the code behave more like you were expecting!

`const Boutique& a` would be better for the function parameter. No reason to make a copy just to print it out. — Retired Ninja, Apr 15 '21 at 00:00

score 0 · Answer 2 · answered Apr 15 '21 at 00:49

There are numerous problems with this code.

In Addarticle(), typeid(*a) will always be equal to typeid(Article). The correct way to check if a is pointing at an object of a particular derived type is to use dynamic_cast instead, eg:

if (dynamic_cast<ArticleEnSolde*>(a))
{
    // a is an ArticleEnSolde ...
}
else
{
    // a is not an ArticleEnSolde ...
}

Also, in static_cast<const ArticleEnSolde&>(*arts[arts.size()]), accessing arts[arts.size()] is going out of bounds of the vector. You likely meant to cast a instead.

However, in this situation, you should use the clone() idiom instead, if you are not going to push a itself as-is into the vector:

class Article
{
public:
    virtual Article* clone() const;
};

class ArticleEnSolde : public Article
{
public:
    Article* clone() const override;
};

...

Article* Article::clone() const
{
    return new Article(*this);
}

Article* ArticleEnSolde::clone() const
{
    return new ArticleEnSolde(*this);
}

void Addarticle(Article* a)
{
    Article* p = a->clone();
    arts.push_back(p);
}

Regarding printing, you are invoking operator<< only for Article because that is the type of object you are passing to it. It has no way to know whether the object is actually an ArticleEnSolde or not. For polymorphic types, you should instead define a virtual method in the base class, and then define operator<< for that class and have it call the virtual method:

class Article
{
public:
    virtual void print(ostream &n) const;
};

class ArticleEnSolde : public Article
{
public:
    void print(ostream &n) const override;
};

ostream& operator<<(ostream& n, const Article &a);

...

void Article::print(ostream &n) const
{
    n << "\narticle not on sale\n";
}

void ArticleEnSolde::print(ostream &n) const
{
    n << "\narticle on sale\n";
}

ostream& operator<<(ostream& n, const Article &a)
{
    a.print(n);
    return n;
}

ostream& operator<<(ostream& n, const Boutique &a)
{
    n << "\nshowing boutique:\n";
    for (size_t i = 0; i < a.arts.size(); ++i)
    {
        n << *(a.arts[i]);
    }
    return n;
}

how to cout a subclass after static cast into a vector

2 Answers2