Skip to main content
Code Review

Return to Revisions

2 of 2
Commonmark migration

Looking at the code you've posted here, it's pretty straightforward and I'm able to understand what it does. That's great! I have a few suggestions on how to improve it.

Named Types

One common complaint about the standard template library and templates in general in C++ is the ridiculously long type names that result from using them. I see that in your code here. It's easily solved by creating new named types that represent the common types you use. For example:

std::vector<std::shared_ptr<sf::Drawable>>

uses 3 types with namespaces. If you did this:

typedef std::shared_ptr<sf::Drawable> sharedDrawable;

Then your new type becomes:

std::vector<sharedDrawable>

Or even better, use the new using directive in C++11:

using sharedDrawableVector = std::vector<std::shared_ptr<sf::Drawable>>;

Or something along those lines.

Use C++!

You're writing in C++, but you're using older methods of iterating through containers. In the destructor of Element you're going through the list manually when you could use a std::find() method instead.

Magic Numbers

In the constructor of Element you create a vector with coordinates <49, 49>. What does it mean? What is it? Why <49, 49>? You should create a constant with a name that describes what this represents:

const sf::Vector2f kRectOriginVector {49, 49}; // Or whatever it means

Then use that in the constructor.

user1118321
  • 11.9k
  • 1
  • 20
  • 46
default

AltStyle によって変換されたページ (->オリジナル) /