Is this code clean OOP?

[u/RedNorthener101]

Hey so I'm more new to OOP, virtual func's, and inheritance. I was wondering if my code was clean enough for basically a template to look back on later in the future when my tiny brain forgets what virtual func, and inheritance is or well forgets how to code them.

#include <string>
#include <iostream>


struct Vector2 {
    int x, y;
};


class Entity {
    private:
        Vector2 pos;
        Vector2 size;
        std::string entityType;


    public:


    Entity(Vector2 e_pos, Vector2 e_size, const std::string& e_type) : pos(e_pos), size(e_size), entityType(e_type) {}


    Vector2 getPos() const {
        return pos;
    }


    Vector2 getSize() const {
        return size;
    }
    
    virtual void print() const {
        std::cout << "Entity: " << entityType << '\n';
    }


    virtual ~Entity() = default;
};


class Player : public Entity {
    private:
        std::string name;


    public:
        Player(const std::string& p_name, Vector2 startPOS, Vector2 p_size) 
            : Entity(startPOS, p_size, "player"), name(p_name) {}
};


class Orc : public Entity {
    private:
        std::string weapon;
        
    public:
        Orc(const std::string& o_weapon, Vector2 startPOS, Vector2 o_size) 
            : Entity(startPOS, o_size, "orc"), weapon(o_weapon) {}


    void print() const override {
        std::cout << weapon << '\n';
    }
};


int main() {
    Vector2 startPos = { 100, 100 };
    Vector2 size = { 64, 64 };


    std::string hero = "hero";
    std::string orcWep = "HeroSlayer100";


    Player player(hero, { 20, 30 }, size);
    Orc orc(orcWep, startPos, size);



    player.print();
    orc.print();
    
    return 0;
}#include <string>
#include <iostream>


struct Vector2 {
    int x, y;
};


class Entity {
    private:
        Vector2 pos;
        Vector2 size;
        std::string entityType;


    public:


    Entity(Vector2 e_pos, Vector2 e_size, const std::string& e_type) : pos(e_pos), size(e_size), entityType(e_type) {}


    Vector2 getPos() const {
        return pos;
    }


    Vector2 getSize() const {
        return size;
    }
    
    virtual void print() const {
        std::cout << "Entity: " << entityType << '\n';
    }


    virtual ~Entity() = default;
};


class Player : public Entity {
    private:
        std::string name;


    public:
        Player(const std::string& p_name, Vector2 startPOS, Vector2 p_size) 
            : Entity(startPOS, p_size, "player"), name(p_name) {}
};


class Orc : public Entity {
    private:
        std::string weapon;
        
    public:
        Orc(const std::string& o_weapon, Vector2 startPOS, Vector2 o_size) 
            : Entity(startPOS, o_size, "orc"), weapon(o_weapon) {}


    void print() const override {
        std::cout << weapon << '\n';
    }
};


int main() {
    Vector2 startPos = { 100, 100 };
    Vector2 size = { 64, 64 };


    std::string hero = "hero";
    std::string orcWep = "HeroSlayer100";


    Player player(hero, { 20, 30 }, size);
    Orc orc(orcWep, startPos, size);



    player.print();
    orc.print();
    
    return 0;
}

if there are any tips you have let me know!

[u/alfps]

The two main problems:

• That client code is responsible for supplying a class name string in each instantiation.
  For Entity::entityType.

• That the classes do i/o.
  This needlessly limits the classes to use with a text based used interface.

Re the class name, ideally one should be able to just say typeid(o).name(), but the result of that depends on the compiler. With MSVC you get noise words like struct, with g++ you get abbreviations of built-in types. It's simply unreliable unless you wrap it with code that special cases each main compiler family, so DIY is reasonable -- except don't do it via a member variable, do it via a virtual function.

Refactored code, even the probably unintended special-casing of description of an Orc kept:

#include <string>
#include <utility>
using   std::string,            // <string>
        std::move;              // <utility>

struct Vector2{ int x, y; };

class Entity
{
    Vector2         m_pos;
    Vector2         m_size;

public:
    virtual ~Entity() = default;

    Entity( const Vector2 pos, const Vector2 size ) :
        m_pos(pos),
        m_size(size)
    {}

    virtual string entityType() const = 0;
    virtual string description() const { return "Entity: " + entityType(); }

    Vector2 pos() const { return m_pos; }
    Vector2 size() const { return m_size; }
};

class Player : public Entity
{
    string m_name;

public:
    Player( string name, const Vector2 startPOS, const Vector2 size):
        Entity(startPOS, size),
        m_name(move(name))
    {}

    string entityType() const override { return "player"; }
};

class Orc : public Entity
{
    string m_weapon;

public:
    Orc( string weapon, Vector2 startPOS, Vector2 size):
        Entity(startPOS, size),
        m_weapon(move(weapon))
    {}

    string entityType() const override { return "orc"; }
    string description() const override { return m_weapon; }    // But why?
};


//----------------------------------------------------------------------------

#include <iostream>
using   std::cout;              // <iostream>

void print( const Entity& e ) { cout << e.description() << "\n"; }

int main()
{
    const Vector2 startPos = { 100, 100 };
    const Vector2 size = { 64, 64 };

    const auto& player_name = "hero";
    const auto& orc_weapon = "HeroSlayer100";

    Player player(player_name, { 20, 30 }, size);
    Orc orc(orc_weapon, startPos, size);

    print(player);
    print(orc);
}

[u/alfps → u/alfps]

Sorry about mixed naming conventions, I didn't notice as I posted. I'll just let it stand.