Review: std.logger
Sönke Ludwig via Digitalmars-d
digitalmars-d at puremagic.com
Sat Jul 12 09:13:28 PDT 2014
Overall looks good to me. Some points that haven't been mentioned so far
in this review round:
- Using a class with static members doesn't seem to be very idiomatic.
It seems like the three member properties can simply be made global and
everything should be fine - the "LogManager." prefix doesn't really add
information. This has been mentioned in the last review round and it's
not a very important point in this particular instance, but we should
really make a decision here that will also decide how future modules go
about this.
- Personally, I really find additional verbose log levels useful.
Currently there is only "trace". Previous proposals had a generic
"verbose(N)" set of levels, but I think it's important for the proper
interaction of multiple libraries to define a semantic set of verbose
levels. A proposal, which has worked very well me (from low to high):
trace:
as now, used for low level tracing of program flow
debugv:
verbose debug messages, may flood the log
debug:
normal, low frequency debug messages, useful for the developer
diagnostic:
diagnostic output also potentially useful for the user
this is what you typically would get with the -v command line switch
- There is a "formatString" string mixin that includes a lot of
if-else cases that seem to do exactly what formattedWrite would do
anyway, is there something that it actually does in addition to that?
Also, why is it a string mixin in contrast to a simple (template)
function? It may be a matter of taste (but also of compile time/memory),
but I'd almost always prefer a non-string-mixin solution.
- Even if it may be more typing (or maybe not?), actually writing out
the different signatures for each log level and the c/f suffixes would
be very advantageous for the documentation and for code completion. It
would also make the EBNF unnecessary, which I agree with Johannes looks
a little scary. All of the functions could be based on the generic
logl/loglf functions, so that there wouldn't be much redundancy.
- I'm still really not sure if the "c" versions of the log functions
pull their own weight. They seem to be in line with trivial convenience
functions, which are generally discouraged in Phobos (with the same
issues, such as additional cognitive load and bigger API size).
- The functions error(), info(), fatal(), etc. don't follow the usual
rule for functions to start with a verb. The question is if saving three
characters over logError() is worth making the code more ambiguous for
the outside reader (e.g. "does error() throw an exception? or set some
internal error state?" "does fatal() terminate the process?")
More information about the Digitalmars-d
mailing list