Skip to content
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

Cppcheck fixes #1760

Merged
merged 3 commits into from Sep 24, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

Correct a warning from cppcheck:

binary_writer.hpp:869: (style) Consider using std::accumulate algorithm instead of a raw loop.

https://github.com/Xav83/nlohmann-json-cppcheck/commit/910a7d2b873dd7ae92ec81cced2bf73200ff4848/checks#step:5:107

Signed-off-by: Xav83 <x.jouvenot@gmail.com>
  • Loading branch information...
Xav83 committed Sep 19, 2019
commit 13a7c602578878bd524da551a1005b92e8d8e79b
@@ -861,13 +861,12 @@ class binary_writer
*/
static std::size_t calc_bson_array_size(const typename BasicJsonType::array_t& value)
{
std::size_t embedded_document_size = 0ul;
std::size_t array_index = 0ul;

for (const auto& el : value)
const std::size_t embedded_document_size = std::accumulate(std::begin(value), std::end(value), 0ul, [&array_index](std::size_t result, const typename BasicJsonType::array_t::value_type & el)
{
embedded_document_size += calc_bson_element_size(std::to_string(array_index++), el);
}
return result + calc_bson_element_size(std::to_string(array_index++), el);
});
This conversation was marked as resolved by nlohmann

This comment has been minimized.

Copy link
@jaredgrubb

jaredgrubb Sep 20, 2019

Contributor

Doesn't the lambda need to be mutable (so array_index can mutate)? If so, I'm curious how this passes unit tests; I would hope this would get checked?

This comment has been minimized.

Copy link
@Xav83

Xav83 Sep 20, 2019

Author Contributor

Indeed I've forgot to make it mutable !
This is weird, I run the commands describe in the README and everything passed on my machine. At least the CI catched it

This comment has been minimized.

Copy link
@Xav83

Xav83 Sep 20, 2019

Author Contributor

I may have runned the tests before using the command make amalgamate. So this is why the tests passed on my machine before 😂
I've removed the indentifier initializer and the auto keyword in the lambda parameter since those are c++14 features, and if I understand well, this project is in c++11


return sizeof(std::int32_t) + embedded_document_size + 1ul;
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.