Why are the branches of the if statement not implemented consistently?
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)!