nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
42.4k stars 6.67k forks source link

Capture exceptions by const& in docs. #4099

Closed iwanders closed 1 year ago

iwanders commented 1 year ago

Hi,

The core guidelines recommend catching by const& for all but very small value types. One of my colleagues filed a PR internally with catch (nlohmann::json::type_error& e), instead of catch (const nlohmann::json::type_error& e), with the explanation that it was copied from the documentation. The error here holds a string however, so it is not a simple type that fits in a single register, I think const & is appropriate. This PR updates the documentation's exception catching to be all const.

Automated replace performed with the following command:

find ./ -type f -name *.cpp -print0 | xargs -0 sed -i -e 's|catch (json|catch (const json|g'

Pull request checklist

Read the Contribution Guidelines for detailed information.

coveralls commented 1 year ago

Coverage Status

coverage: 100.0%. remained the same when pulling e5fb288b90ae6e6302bfd115a4417760a3e11c81 on iwanders:catch-by-const-in-docs into 5d2754306d67d1e654a1a34e1d2e74439a9d53b3 on nlohmann:develop.

iwanders commented 1 year ago

Failure here seems unrelated to these changes?

Use of uninitialized value $Clang in concatenation (.) or string at /usr/bin/scan-build line 1859.
sh: 1: : Permission denied
scan-build: error: Cannot find an executable 'clang' relative to scan-build. Consider using --use-analyzer to pick a version of 'clang' to use for static analysis.
gmake[3]: *** [CMakeFiles/ci_clang_analyze.dir/build.make:72: CMakeFiles/ci_clang_analyze] Error 1
gmake[2]: *** [CMakeFiles/Makefile2:1143: CMakeFiles/ci_clang_analyze.dir/all] Error 2
gmake[1]: *** [CMakeFiles/Makefile2:1150: CMakeFiles/ci_clang_analyze.dir/rule] Error 2
gmake: *** [Makefile:465: ci_clang_analyze] Error 2
Error: Process completed with exit code 2.
nlohmann commented 1 year ago

Thanks!