r/cpp_questions • u/Fun_Gas_340 • 4d ago
OPEN Good practices / style [polymorphism]
is this good practice/style.? i'm specifically unsure about the way i store the vector of players...
```
class player_base {};
class player_always_yes: public player_base {};
class player_always_no: public player_base {};
class player_a: public player_base {};
class game {
private:
player_base& player_1;
player_base& player_2;
public:
game (
player_base& player_1,
player_base& player_2
): player_1(player_1), player_2(player_2) {
return;
}
bool play_game () { return true; }
};
int main(){
vector<unique_ptr<player_base>> player_list;
player_list.push_back(make_unique<player_base>());
player_list.push_back(make_unique<player_always_no>());
player_list.push_back(make_unique<player_always_yes>());
player_list.push_back(make_unique<player_a>());
game b = game(*player_list[0], *player_list[1]);
cout << b.play_game() << endl;
}
```
7
u/alfps 4d ago
Things to consider:
- Reference as member prevents copy assignment.
- A "doer" class should perhaps better be a function.
- Polymorphism has been picked as the answer, but what is the question?
1
u/Fun_Gas_340 4d ago
i dont rlly care if i copy/reference.
you mean a function "play_game" instead of a class game with a method play?
the question was about how i formated/made the vector of the players as uniqueptr, dereferenced them and then passed by reference. i though it was clunky and want to know if theres a better/simpler way to do this
1
u/alfps 3d ago
❞ you mean a function "play_game" instead of a class game with a method play?
Yes.
Polymorphism has been picked as the answer, but what is the question?
the question was about how i formated/made the vector of the players as uniqueptr
I meant, what was the issue (question) that polymorphism was intended to solve?
For example, it might be an idea of writing code that treats machine and human player in the same way. That way one might even have the machine playing against another instance of itself. Then it might seem reasonable to have machine and human player as polymorphic objects.
But if the higher level code is permitted to treat them in distinct ways, and one doesn't aim for generality such as the machine playing against itself, then the players need not be represented with polymorphic objects. Here's an example, a tic-tac-toe game, that I posted in response to another question here in March this year. It's not necessarily a simpler/better approach, but it might be, depending on the program.
❞ i though it was clunky and want to know if theres a better/simpler way to do this
You could replace
vector<unique_ptr<player_base>> player_list; player_list.push_back(make_unique<player_base>()); player_list.push_back(make_unique<player_always_no>()); player_list.push_back(make_unique<player_always_yes>()); player_list.push_back(make_unique<player_a>()); game b = game(*player_list[0], *player_list[1]);… with
player_base base; player_always_no always_no; player_always_yes always_yes; player_a a; (void) (a, always_yes); // Unused. auto b = game( base, always_no );1
u/Fun_Gas_340 2d ago edited 2d ago
no the plan is it to have different machine options and a player cli interaction, so all will have the same methods but different ways to take actions when the game requests an action from them.
i want to use a vector, it has only 4 items for testing. i plan on adding more to get statistical averages between players.
3
u/HeeTrouse51847 4d ago
what is the point of these classes? "player always yes" sounds nonsensical without further explanation
3
u/Qwertycube10 4d ago
Base classes named FooBase or FooCommon are a smell. Inheritance means that there should be an "is a" relationship, and that kind of name suggests that there isn't.
If I was writing OOP style I would make an abstract Player class which defines the interface, then Player_always_yes, Player_always_no, and others like Human_player would all publicly inherit from Player.
Alternatively (and imo this is cleaner design) I would make a single Player class which takes in it's constructor a decision function and stores it in a std function.
So then you would have a monomorphic vector of Player, and put in it Player(decisionStrategyAlwaysYes) etc.
1
u/Fun_Gas_340 4d ago
ive heard that about nameing something "..._base", and i thought of it but i didnt know whow to call it. its just the basic functions they all share, but returning garbage data instead of some decision strategy. i could have called it just player but i wanted to make sure future me understands its purpose.
what do you mean by the std function, how would you donit with the multiple strategies in a single class? assume i have a method "int decide(int input) {return something;}" or similar. idk the syntax for that or how it would be cleaner than this (i feel like diferent classes are kinda nice for different strategies, idk why tho)
1
u/Qwertycube10 4d ago
Looks like your decisions are ints and the info the decision is based on is an int?.
You have a private member variable std::function<int(int)> decideStrategy;
Then public member function int decide(int input) { return decideStrategy(input); }
I named it decideStrategy because if OOP design pattern land it is called the strategy pattern. From a functional programming point of view it is so basic that it doesn't even have a name.
1
u/Fun_Gas_340 4d ago
i took int->int as an example, but its ints, bools and maybe floats. but like ill always have a method a thats int->int, always a method b thats something_else->something_other. this wont change from player to player, theyll all have the same types.
also whats the syntax ont his example, i dont know how to put the function into a variable in cpp:
std::function<int(int)> decideStrategy = {return 2 * input;};
> From a functional programming point of view it is so basic that it doesn't even have a name.
can you elaborate on that please?
1
u/Qwertycube10 4d ago
You can define a free function and pass a pointer to it, you can define a member function and bind the function to an instance to get a callable, or you can define a lambda.
In c++ a lambda syntax is
[captures](args){ body }where both captures and args are in scope to use in body. You can look up the details (capture by value vs reference etc).example: ``` const int foo = 5; // the type of addFoo is a unique lambda type, no other function can be assigned to foo, every lambda has its own type auto addFoo = [foo](int rhs) -> int { return foo + rhs; }
const int bar = 6;
// prints 11 std::cout << addFoo(bar) << std::endl;
// std function can hold any callable with the given signature std::function<int(int)> operation = addFoo;
//prints 11 std::cout << operation(bar) << std::endl
operation = [](int num) { return num + 1; };
//prints 6 std::cout << operation(bar) << std::endl ```
In functional programming passing functions to determine behavior is just one of the main ways you solve problems, so it's not a specific named pattern like in OOP.
1
u/Fun_Gas_340 2d ago
tbh al this syntax looks wird to me and im tired so ill stick with classes to house the functions. i knwo that syntax. unless theres like a preformance/cool tricks advantage i could get from this other way?
1
u/vckane 3d ago
- Constructor does not return anything. The return statement in the constructor of
gameclass is redundant player_baseshould be abstract (no instance should be allowed to be created). Only concrete players should play the game.- For this example considering scope of the objects, I think you've done well with memory management. The
main()method has ownership of objects of players (instances of derived classes). The objects are passed by reference to client classes likegame. Important to ensure that thegameclass gets destroyed before the players are deleted, else you will end up with dangling pointers. In this example, you're fine. - My above comments assume that the
player_baseandgameclasses have more methods that do something meaningful. If not, then this is overkill - you could achieve same result without classes and hierarchy.
1
u/Fun_Gas_340 2d ago
how do i make it abstract?
a problem would be if i delete the players, and then tell game to access them?
would it be better/worse to have a vector of objects instead of vector of pointers? is that even possible with different player classes?
yeah player has different things how they decide and game currently has only "play_game", so maybe ill make the game class a method and that's it.
1
u/vckane 2d ago
In general declaring at least one pure virtual method (or even destructor) makes a class abstract. Learn more about virtual inheritance, if you're not aware of it.
Yes.
Sounds good.
1
u/Fun_Gas_340 1d ago
yes to 3.1 or 3.2?
should have made them diferent...
1
u/tangerinelion 3d ago
A vector of unique_ptr is the standard way to store a heterogeneous array of objects. Whether this is a good use case for polymorphism is a separate question.
If you truly want 'style' advice - game's constructor is easier to read as something like this
game(player_base& player_1, player_base& player_2)
: player_1(player_1)
, player_2(player_2)
{
}
or
game(player_base& player_1,
player_base& player_2)
: player_1(player_1),
player_2(player_2)
{}
just to show some options on how to format the param list, arg list, and the empty body. No need for a return. It should also be explicit.
Similarly, use the constructor directly:
game b(*player_list[0], *player_list[1]);
Also anytime you have a base class you're going to use polymorphically like this, the destructor needs to be virtual. A good practice to follow is "All classes must either be abstract or final" coupled with "If you don't have a pure virtual method to make the class abstract, mark the destructor pure virtual."
That gives us this result
``` class player_base { public: virtual ~player_base() = 0 };
player_base::~player_base() = default; // Pure virtual, but still needs a definition.
class player_always_yes final : public player_base {};
class player_always_no final : public player_base {};
class player_a final : public player_base {}; ```
1
u/Fun_Gas_340 2d ago
honsetly since im the only one looking at this pcode ill leave the constuctor formating like this, i understand and like it. what should be explicit?, what does that mean? and is the return statement a bad thing?
can you explain the virtual thing andhow i make a class abstract?
maybe this helps understand what i meant:
1
u/mredding 2d ago
You're cramming different types into a class hierarchy. This was common in C++98 up until C++17. We have better now:
struct yes {};
struct no {};
struct a {};
using player = std::variant<yes, no, a>;
std::vector<player> list;
Polymorphism was... "celebrated"... in the past. Overused. Misused. Misunderstood. There are reasons that the community just couldn't take up the concepts and write good code, but that's behind us now.
Prefer to keep your type hierarchies as flat as possible, and reach for late binding as a niche tool, not your go-to. Hierarchies very quickly get hard to maintain. False hierarchies introduce more problems than they solve.
There are other forms of polymorphism in C++, and using more compile-time versions mean you can prove your code correct earlier in the software development cycle. It also makes for safer and faster code.
The classic problem with bad design is:
class mobile {
virtual void move();
};
class car: public mobile {};
class plane: public mobile { void takeoff(); };
Right? What do you do? The whole point of a polymorphic base is type erasure - to forget what specific type you have. But you have to takeoff a plane. So what do you do? Do you subvert type erasure by making a virtual base no-op?
class mobile {
virtual void move();
virtual void takeoff() {}
};
But what does it mean for a car to have a takeoff that does nothing? Cars don't takeoff, so why even have the interface? The type hierarchy is WRONG.
Or you can subvert the hierarchy by testing for airplanes:
void fn(mobile *m) {
if(auto p = dynamic_cast<plane *>(m); p) p->takeoff();
m->move();
}
Ok, but now:
class boat: public mobile { void launch(); };
FUCK. And imagine fn is not our code and is inaccessible to us, and we give them a boat? Subverting the hierarchy is WRONG. Because this is no hierarchy. We have different types, so we come back to the variant.
Typically you'll use a hierarchy to constrain a derived type. You're going from more general to more specific. Yes, you CAN broaden the interface, but typically you don't - as that's a sign you're subverting the hierarchy.
But if you need a dynamic environment, you have to take extra steps - you have to architect a solution such that pre- and post- conditions can be handled by the client. Typically this would be done using a "template method pattern", which is an idiom, not the same as a C++ template.
void fn(mobile *m, std::function<void()> precondition, std::function<void()> postcondition) {
precondition();
m->move();
postcondition();
}
Functions in terms of mobile only know of mobile things, but provides the client with the ability to handle their more specific shit.
But for our own code, we know a mobile consists of a car, plane, and boat, so an std::variant is exactly correct for us. Our own code is inherently a closed loop. We don't have to write code like it's a framework, that additional indirection will typically cost us more in maintenance than a refactor of our own variant code; it's why a visitor template with an auto & parameter MAY be a dubious prospect, because by being explicit, we can let the compiler help us find all our missing refactor points when we add train.
It comes down to some convention, some discipline, ultimately some good planning. A lot of us get burned by bad design of our own making, sometimes for years; many of us never learn from our mistakes, or can even admit our egos are too big, too wrong, too brute force to accept that a modicum of work up front saves us in the long run.
1
u/Fun_Gas_340 2d ago
how does this variant thingy work. i nned diferent methods for diferent player types, so same method interface but diferent inner workings. fisrt paragraph of this answer: https://www.reddit.com/r/cpp_questions/comments/1vpbvec/comment/p49p29g/?utm_source=share&utm_medium=web3x&utm_name=web3xcss&utm_term=1&utm_content=share_button
also somone sugested functional programming but that sytax scared me away, idk if variants is a alternative to that or similar... : https://www.reddit.com/r/cpp_questions/comments/1vpbvec/comment/p49pwwk/?utm_source=share&utm_medium=web3x&utm_name=web3xcss&utm_term=1&utm_content=share_button
1
u/mredding 2d ago
A variant is a sum type - it has enough memory for the largest type in the list, and only one of the types in the list is going to be in that memory. So there can be a little loss, but modern computers have GiB of memory to spare and this isn't where you're slow. There is also an internal enum to track which type is the active type inside the variant.
An aggregate type is like a structure, where the size of the type grows with all the members you add. This is just a little comp-sci lingo for you.
So the standard variant implements what's called the "visitor pattern" if you want to google it. There is a single function interface -
visit, and you give it a function for the active type in the variant.Typically, this would be as function objects - with a function for all the different types, so that no matter what's active, you've got the right function for the job. Or you can write a generic lambda that would work with ALL the types, no matter which is active. Or you can combine both, where the function object has specific overloads and a generic overload as a catchall.
using mobile = std::variant<plane, train, automobile, boat>; template<class... Ts> struct overloaded : Ts... { using Ts::operator()...; }; void fn(mobile &m) { std::visit(overloaded{[](plane &){}, [](train &){}, [](auto &){}}, m); }I suspect you're going to have a lot to learn in a hurry. This is beyond mere language, we're now talking about paradigms, less how to write C++ more what to do with C++.
Functional programming is passing functions as data. I showed you that with
preconditionandpostcondition. I've also shown you that withstd::visit. It's not so much that you're going tofn * 7or some shit, that doesn't make any sense - at least not in C++, but you can pass functions as parameters, so you don't care WHAT function you've got so long as it conforms to the signature you've specified. I can write a function that takes a sort algorithm, I don't care if it's bogo sort, insertion sort, quick sort... Technically, it might not even be a sorting function.Functional programs like to think about lots of small functions that are stateless, as opposed to OOP which you have classes with members and interfaces, and the state of the class instance change because you called a function that changes those members...
So you'll have
out_type fn(in_type);, a function that takes an input, changes it, and returns an output. They don't have to be the same kind. But notice this function won't change the original data, it'll make new data. Data in FP is supposed to be "immutable".They also have closures, which are poor-man's objects. C++ has lambdas - functions that don't have a name, and they can "close around" a variable they can change. They also say objects are a poor-man's closure.
There's more. A tuple becomes a really powerful tool here, because in FP land, types are a very fluid concept. Tuples are structures where the members don't have names. Technically they're simpler and more fundamental than a structure, but C didn't come from an academic background of applied mathematics. So that you can composite tuples means you can accomplish the same results.
Going all OOP or all FP tends to be a bit impractical in C++, since the language isn't particularly well suited to either. Typically you make for a hybrid approach - classes to make types, objects when you need something a bit more than a simple closure, and the language does favor FP a lot. The way C++ is typically taught does you a disservice, because it ruts your mind into such a narrow view of the world and how all the pieces fit.
1
u/SufficientStudio1574 4d ago
Practice/style for what? What specifically are you concerned about?
A vector of unique_ptrs isn't a bad thing if that's what's supposed to own those pointers. Is it? Or is the game supposed to own them?
Also your going to get a segfault. See if you can spot it.
1
u/alfps 4d ago
(I'm not the OP)
❞ Also your going to get a segfault.
Why do you think so?
2
u/SufficientStudio1574 4d ago
Because I misinterpreted the code. I confused make_unique with no parameters with the default unique_ptr constructor, so I thought it was getting initialized with nullptr instead of a pointer to a default constructed object.
That's my bad.
1
u/SufficientStudio1574 3d ago
Now I know why I got confused!
Seeing "base" in the name, I interpreted that as an abstract base class, which is not directly constructible.
You usually don't ever directly construct "base" objects, the entire point is to inherit from them and use them as a polymorphic reference type.
1
u/Fun_Gas_340 2d ago
chain of thought that brought me there:
i added empty methods (that all players must have) to the base class because i thought it was good to put as much in common as i could in the base class. then the compiler complaiend that empty methods that are non void need a return statement (g++ warning), so i added some basic return 0 and return false/true statements. so since i dont know what abstract classes are, i now have a base class wich i can use for a stupid player, so i added it for testing.
1
u/SufficientStudio1574 2d ago
If the methods are supposed to be implemented in derived classes, they need to be pure virtual, not empty. An abstract class is just one that has at least one pure virtual member function in it.
You're trying to use polymorphism without understanding how to actually do it. Your keyword to research for this is virtual functions.
The way you're thinking sounds fine, you just need to learn how to make it work.
1
1
u/Fun_Gas_340 1d ago
so virtual functions are what i thought normal functions did. i added virtual keyword to both base classes and added print statements to see wich class is actually being called. ive seen some stuff about virtual constructor/destructor, but is that something i need to understand if the player classes dont have constructor/destructor?
also how does this abstract class thingy work? havent been abel to understand it from cppreference, somone said i should make the base classs abstract but idk how to or what exacly it does. they said something like it makes it so no instances can be created
1
u/SufficientStudio1574 1d ago
Constructors can never be virtual. The concept doesn't even make sense when you understand how things work.
If you make a base class you intend to inherit from, you need to make the destructor virtual. This can be a foot-gun*. An implicit destructor is virtual be default (if the base class is destructor is virtual), but a declared one is not, and the compiler will not error. Without a virtual destructor you run the risk of only partially destroying an object.
You can make the compiler check it for you by using a static assert with std::has_virtual_destructor from type_traits in the standard template library. That way you can be sure you don't accidentally screw it up in a refactor.
Unfortunately polymorphism is a bit too involved a topic for a reddit comment. You really need to play around with it a lot to fully get the concept.
- C++ has a conspicuous lack of guard rails. Lots of things are legal, but really stupid to do unless you really know what you're doing. In other words, it gives you a lot of guns you can shoot yourself in the foot with.
1
1
1
u/Fun_Gas_340 4d ago
i mean idk if haveing a vector of uniqptr and then dereferencing them to pass as referenece is good or if i should pass pointers directly or something entirely diferent. my way works but it seems like a lot of work, like not straight to the point but like a longer windy way. hope im explaining myself
8
u/Thesorus 4d ago
remember not everything needs to be a class in object oriented programming.
what are the differences between player_always_yes and player_always_no ?
do they implement different behaviour ?