Review: std.logger

[Robert burner Schadek via Digitalmars-d]

On Thursday, 24 July 2014 at 18:51:03 UTC, Andrei Alexandrescu 
wrote:
>
> 0. There's no way to set the minimal logging level statically, 
> except for an on/off switch. There must be a way to define e.g. 
> -version=logLevel=trace that sets the minimum logging level 
> actually performed. Any logging below that level is a no-op. 
> Which segues into the next problem:

I'm currently working on this.

>
> 1. There's a bunch of code still generated even with logging 
> statically disabled (per http://d.godbolt.org). I could not 
> find a way to completely eliminate generated code. Unused lazy 
> parameters MUST generate no code at the front end level. This 
> is a performance bug in the D front end that blocks acceptance 
> of this proposal.

that is part of 0. work

>
> 2. The three one-letter suffixes don't seem to scale well. They 
> optimize for brevity but at the cost of clarity. Also they 
> establish a precedent - are we sure we recommend future D code 
> in the standard library and beyond to mangle names using 
> one-letter conventions? I think we must find a way to solve 
> things via overloads.
>
> Currently we have: log, logc, logf, logl, logcf, loglf, loglc, 
> loglcf. One would almost expect logwtf to be somewhere in there.
>
> I think an overloading-based solution would be a better choice 
> here. First, I think the logging level must guide all 
> overloads. True, we'd sometimes like to log the logging level 
> itself, but that's easily done by using a formatting function 
> (as shown below). So:
>
> log(A...)(lazy A stuff); // just log whatevs
> log(A...)(LogLevel lvl, lazy A stuff); // log at specified level
> log(A...)(bool c, LogLevel lvl, lazy A stuff); // conditionally 
> log at specified level

log(A...)(lazy A stuff) matches the next two sigs as well. I 
tried that.
Maybe some overloading inside the function body may work, but 
that is just a mess IMo.

>
> Then three more logf with similar signatures, using the 
> precedent set by write/writef. And we're done.
>
> To log the logging level itself, just use logf(someLevel, "%s", 
> someLevel) and be done with it.
>
> There's no conditional logging without specifying a level, 
> which should be fine seeing as conditional logging is not that 
> frequent to start with.

there is. tracec, infoc, ....

>
> There should be some shortcuts for logging levels such that one 
> can write log(info, "crap") instead of log(LogLevel.info, 
> "crap").


there is trace%s, info%s, warning%s .... c|f

>
> 3. I'm not sure I like the design using defaultLogger in 
> conjunction with free functions using it. It clearly makes for 
> comfortable casual logging by just calling log(whatevs) and it 
> uses a precedent set by stdout. But I wonder if it would be 
> cleaner to just give it a shorter name "log" and then have it 
> have a "write" method:
>
> log("crap");
> -> becomes ->
> log.write("crap");
>
> Also there'd be log.writef("%s", "crap") etc.

well, this is by design. I wanted to provide very easy simple 
looging for hacking a small script. If you want more, you 
properly want to handle Loggers as variables.

>
> 4. Conditional logging must be justified. In my mind the 
> justification is that statically setting the log level makes 
> the code disappear without the condition being ever evaluated, 
> but the current design doesn't allow setting the log level!
>
> 5. There was some nice stuff in the previous std.logger work by 
> me and later Jose (I think), which allowed logging every n 
> times/milliseconds so as to allow nice throttling. That's nice 
> to omit/defer for simplification purposes, but I noticed that 
> log noise is a serious matter.

I could start std.logger.condition

>
> 6. The current backend design requires use of classes and 
> references, i.e. garbage collection. Amid the current tendency 
> to make std work without requiring GC, I think a design based 
> on RefCounted would be recommended here.

Maybe I'm wrong, but RefCounted does not support polymorphism and 
that would break not only the MultiLogger and the defaultLogger. 
I think this is a legitimate use of classes, as Logger properly 
stay alive the complete run of the program.

>
> 7. The current backend design fills a struct with data then 
> passes it to the implementation. But if the implementation 
> doesn't use e.g. the timestamp then that work has been wasted. 
> Maybe offer the fields as properties instead, with caching upon 
> first use?

hm, but taking the timestamp after the log call seams wrong. 
Again, I think this is by design from using polymorphism.

>
> 8. Documentation needs work as it has disfluencies and typos.

If have already worked in all of JakovOvrum and you fixes.

>
> 9. I've also posted a bunch of comments to the code at 
> https://github.com/D-Programming-Language/phobos/pull/1500/files
>
>
> Andrei