This outer check is unnecessary because logRecordBatchStats does the same check. I guess you have it here just to avoid the cost of String.format(). Maybe we should add a var args version which does the string.format internally? When reading code, I personally like it when I can read as much of the real function logic in one page, large bodies of logging code make this harder. I realize it is just 2 extra lines per logging location but it makes it more verbose.
That said, if you are using an outer check, it allows you to use the "+" string builder and not use String.format(). String.format() is prone to mismatch errors between the order in the format string and the order in the variable list. This is more of a style comment. I will be ok if you don't change it now. [ Full content available at: https://github.com/apache/drill/pull/1444 ] This message was relayed via gitbox.apache.org for [email protected]
