Early review of std.logger

Dicebot public at dicebot.lv
Mon Nov 4 05:46:56 PST 2013


On Monday, 14 October 2013 at 11:39:52 UTC, Dicebot wrote:
> As `std.logger` is still marked as "work in progress" this 
> thread is less formal that typical pre-voting review. Goal is 
> to provide as much input about desirable `std.logger` 
> functionality and current state and let module author to use 
> this information for preparing proposal for actual review / 
> voting.
>
> Lets unleash the forces of constructive destruction.
>
> ===== Meta =====
>
> Code: 
> https://github.com/D-Programming-Language/phobos/pull/1500/files
> Docs: 
> http://burner.github.io/phobos/phobos-prerelease/std_logger.html
>
> First announcement / discussion thread : 
> http://forum.dlang.org/post/mailman.313.1377180809.1719.digitalmars-d@puremagic.com

Ok, finally making some conclusions.

===================
As a review manager
===================

This what needs to be done before moving to next review stage:

1.

Judging by review comments it is clear that some more efforts 
need to be put into demonstrating how existing platform can be 
used as a standard API for more complex logging functionality. 
Addition of `MultiLogger` is good thing here, but providing 
either separate article on topic or extra module that 
demonstrates enhancing existing system feels like a good way to 
avoid confusion.

That said, I do agree that std.logger itself should not be "all 
batteries included" module as it does contradict overall Phobos 
style. It may be extended in future once we start using 
sub-packages much more but right now having good standard API is 
much more important.

2.

Standard logger must be made thread-safe by default. Out of the 
box safety is very important for standard library.

3.

Making default `log` a conditional logger does not seem to be 
gladly accepted decision. I'd recommend to swap it with `logf` 
and rename to `logIf`.

Naming overall should be reviewed to make sure there is no word 
duplication in typical usage pattern - Phobos does favor plain 
procedural/functional approach as default and D module system 
makes it unnecessary to encode namespaces into names.

====================
Personal preferences
====================

It is a matter of personal taste but any single thing in that 
list will make me vote "No" on logger proposal:

1.

API it too verbose.
I want stuff as short and simple as this:
```
import std.logger;
warn("1");
inform("2");
die("3");
```
I don't want to refer logger class instances explicitly for any 
typical task.

Stuff like `auto logger = new MultiLogger(); 
logger.insertLogger(someOtherLogger);` is redundant beyond absurd 
- just how many times one may insert word "logger" in a single 
sentence? :)

2.

Logging payload needs to include fully qualified module name. 
This is necessary for enabling module-local output.

3.

I am not sure if proper global vs local log separation can be 
done without providing two distinct default instances in 
std.logger (as I have already said, I don't consider solutions 
which imply tracking class instance manually).

4.

Static "namespace" classes are redundant and should be replaced 
with module-scope globals.


More information about the Digitalmars-d mailing list