How to make Flappy Bird with C++

What a great library! Thanks! This should be a lot more popular.

>std::shared_ptr<sf::RenderWindow> window; >std::shared_ptr<sf::Sprite> background, bird, pipeBottom, pipeTop;

None of that should be shared_ptr, in fact they shouldn't be pointers at all.

>auto flappy = std::make_shared<FlappyBird>(); >flappy->run();

That would work just fine as a global in main.cpp

FlappyBird flappy;

Shared ownership is a rare case, so in general using shared_ptr should already raise some suspicion. And in this case there is no reason any of the shared_ptr'ed things in the code need to be on the heap so no reason to even have a pointer in the first place.

Code like this is effectively what you get when you have someone that doesn't know how to write C++ but writes it anyway and uses pointers everywhere because they either read that C++ is pointer heavy (it's not) or have learned C++ through some course that is actually a course on algorithms and only uses C"++" as example language and is hence at best showing shitty C with iostream and classes straight out the 1980s. Then those people find the (correct) advice of using smart pointers instead of (owning!!!) raw pointers and just put shared_ptr everywhere without thinking about if there actually is a shared ownership to be expressed and/or if the heap allocation is even needed.

A shared pointer is one where ownership can be shared across any part of the application. That sounds great initially, as it assures the thing you’re pointing at will still be alive while you need it. In reality it creates a vague notion of ownership, where no one is responsible for object lifetime, because everyone is. This creates a “functional global” in your application per instance of your shared pointer class. The net effect is a more modern looking architectural morass.

An example: `std::shared_ptr<sf::RenderWindow> window;` - your main window really doesn't need multiple owners - it can be singly owned by `main()` or whatever the top level construct is for your game with `std::unique_ptr`. If you need to pass the window to some other function, do it by raw pointer or ref, signaling that "here's this object for you to use, but not own". `shared_ptr` should only be used for pointers that actually have multiple owners. The textures here might be a good example for this - you could implement a simple resource system that drops unused resources with `shared_ptr` + `weak_ptr`.

As for why - it's mostly around signaling intent and protecting against leaks. It's much easier to reason about ownership if you don't have to worry about some object deep inside your engine hierarchy holding a `shared_ptr` to the object that owns it. This can still happen with unique_ptr, but is much less likely.

shared_ptr<> is most useful for multithreaded applications where a pointer can be safely shared amongst many threads, where the last one to go out of scope will call delete on the object.

To do this safely the shared_ptr<> class maintains a reference count which is protected by a mutex. This is pretty inefficient if you never share a pointer across threads.

Better to use unique_ptr<> where the Thing that created it is also the Thing that's responsible for deleting it - after making sure that anything it's shared the pointer with is already safely destroyed (another thread, a collection or some other object).

Overuse of shared_ptr is called Java Disease in reviews.

Most programs don't need to use shared_ptr at all.

Side note, there is also no value in making those member functions "protected".

Yeah, since C++11 (the 2011 standard).

'&' isn't necessary to use auto but you would use it here to avoid making an unnecessary copy of the pipe.

In this case, where it is erasing one of the container elements inside the loop, the range-based loop would not be right.

But the code is busted anyway. After it erases the pipe (which is always at index zero) it then skips the pipe that was at index 1, if there was one.

Correct would be to do the check and erase before entering the loop. Then the range-based loop would better.

The code looks fine to me. If you see the newer C++23 guides like Lospinoso's C++ book, auto, shared_ptr, braces default value initializations are all considered best practices now. The only major issue I see is that OP should have used references instead of shared_ptrs for some of the variables.