open-source-parsers / open-source-parsers/jsoncpp
Why are the branches of the if statement not implemented consistently?
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 8.9k
- Forks
- 2.7k
- Avg merge
- 31m
- Merged PRs (30d)
- 1
Description
Dear JSONCPP developers,
I was looking at the following if statement starting on
https://github.com/open-source-parsers/jsoncpp/blob/69098a18b9af0c47549d9a271c054d13ca92b006/src/lib_json/json_writer.cpp#L734
It took me some time to realize that the rather complex code in the then branch:
https://github.com/open-source-parsers/jsoncpp/blob/69098a18b9af0c47549d9a271c054d13ca92b006/src/lib_json/json_writer.cpp#L738-L757
is equal to joining the elements in the range [0,size) where size != 0 with a separator,
and a functional equivalent, yet simpler, implementation is
for (unsigned index = 0; index < size ; ++index) {
const Value& childValue = value[index];
writeCommentBeforeValue(childValue);
if (hasChildValue)
writeWithIndent(childValues_[index]);
else {
if (!indented_)
writeIndent();
indented_ = true;
writeValue(childValue);
indented_ = false;
}
if (index < size -1) {
*document_ << ",";
}
writeCommentAfterValueOnSameLine(childValue);
}
I assume performance (saving a comparison) is the reason for the extra complexity.
Yet, when looking at the else branch
https://github.com/open-source-parsers/jsoncpp/blob/69098a18b9af0c47549d9a271c054d13ca92b006/src/lib_json/json_writer.cpp#L764-L768
that is not optimized, like the then branch, to
unsigned index = 0;
for (;;) {
*document_ << childValues_[index];
if (++index == size)
break;
*document_ << ", ";
}
So I wonder
- what is more important for jsoncpp code: readability or performance?
- Shouldn't these two branches not be implemented consistently?
Thanks in advance for your answers (and possibly code improvements)!
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the linked branches in src/lib_json/json_writer.cpp around lines 734-768 and compare their handling of child values and separators. Review the issue's readability-versus-performance questions, then establish which implementation convention the project wants; the work is done when the branches are made consistent with that decision.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100