New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace C arrays with std::array
#2951
Conversation
const std::array bytes = {static_cast<char>(value & 0xFF), static_cast<char>(value >> 8)}; | ||
stream.write(bytes.data(), bytes.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm the one who started using std::byte
here. It wasn't the right move since we immediately have to reintrepret_cast
to a char*
. Might as well just keeping using char
to avoid extra casts.
@@ -56,7 +56,7 @@ In Utf<8>::decode(In begin, In end, std::uint32_t& output, std::uint32_t replace | |||
{ | |||
// clang-format off | |||
// Some useful precomputed data | |||
static constexpr int trailing[256] = | |||
static constexpr std::array<std::uint8_t, 256> trailing = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template type was changed from int
to std::uint8_t
for two reasons. First of all using a smaller type saves memory. We don't need numbers bigger than 5
so std::uint8_t
is a more efficient representation. Second, by using an unsigned type it avoids a sign conversion warning when using one of these array members to index into an array. C-style arrays let you index with signed types but std::array
expects a std::size_t
thus causing a compiler warning when trying to index into it with an int
.
Pull Request Test Coverage Report for Build 9057908394Details
💛 - Coveralls |
9ae6f05
to
67200df
Compare
JoystickState m_state; ///< Current state of the joystick | ||
sf::Joystick::Identification m_identification; ///< Identification of the joystick | ||
int m_file{-1}; ///< File descriptor of the joystick | ||
std::array<char, ABS_CNT> m_mapping{}; ///< Axes mapping (index to axis id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ab1da1c
to
cad70bd
Compare
cad70bd
to
1e9bf4f
Compare
Rebased on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question, but maybe you want to tackle that separately.
Description
SFML uses C style arrays quite a lot so I wanted to start chipping away at replacing them with
std::array
. I chose to start with instances where switching tostd::array
was particularly straightforward. Eventually it would be nice to enable the clang-tidy check that forbids C arrays.For more reading on why C style arrays are not ideal, see the C++ Core Guidelines.
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon1-prefer-using-stl-array-or-vector-instead-of-a-c-array