Early review of std.logger

[Sönke Ludwig]

Am 15.10.2013 10:54, schrieb Robert Schadek:
> On 10/15/2013 09:40 AM, Sönke Ludwig wrote:
>> Am 14.10.2013 20:24, schrieb Robert Schadek:
>>> On 10/14/2013 04:44 PM, Sönke Ludwig wrote:
>>>> The same could be said about the global "log" functions, which are
>>>> tightly coupled to that state. I think this is already nicely grouped
>>>> together by the logger module itself, since there is not much else
>>>> in it.
>>>>
>>>> Basically, I just wouldn't consider this style to be particularly
>>>> idiomatic D code, but of course that's just personal
>>>> perception/preference (there is also some precedence using "struct"
>>>> instead of "class" in Druntime). However, if it ends up like this in
>>>> the
>>>> final version, it should get a "@disable this();" to prevent misuse.
>>> It is for ment for phobos not druntime. Anyway structs would mean all
>>> templates and people will scream template bloat. And this would break
>>> the design, which I find to be a valid use of classes and
>>> polymorphisms.  The StdIOLogger can have its default constructor
>>> called IMO.
>>
>> No no, I was talking about the JobManager, not the Logger classes. No
>> templates involved.
> Than I'm not sure what you're referring to.
>

What I meant is just that in Druntime there is something like this:

struct LogManager {
   static void somefunc();
}

instead of

class LogManager {
   static void someFunc();
}

In any case, such a struct/class should also have a member "@disable 
this();" so that it cannot be uselessly instantiated.

>> Maybe I miscommunicated what I want to show by that example. The (d|D)
>>> part is the rename to enum lower case.
>>> The debug log level is given through the LogLevel.Debug, which will be
>>> renamed to LogLevel.debug. I would call the developer the user of the
>>> logger. Maybe log messages can be communicated to the user of the
>>> applicaiton and the developer of the application through a MultiLogger
>>> class.
>>
>> But the statement of mine that you quoted was about debug levels (the
>> case issue is clear)... Also right now there is no "(D|d)ebug" level,
>> so I'm actually not sure about the statement that you want to make.
>> But my example of having different levels for the application user and
>> the developer is mostly important when the application user enables
>> verbose log output to see where things go wrong. In that case things
>> like system error codes and the like would make sense, but a repeated
>> printout of some kind of internal buffer state would hardly help the
>> user - it could, however, help the developer.
>>
> maybe something like:
> auto devLogger = new StdIOLogger(LogLevel.info);
> auto appLogger = new FencySelfWrittenGuiLogger(LogLevel.Warning);
> auto multiLogger = new MultiLogger(devLogger, appLogger);
>
> multiLogger.log("...");
>
> otherwise, I think I don't follow you
>

The log messages are supposed to go to the same destination, just 
filtered by log level. A totally artificial example:

---
void main(string[] args)
{
     logDiagnostic("Application called as %s", args[0]);
     ubyte[] contents;
     try {
         logTrace("Going to read file");
         contents = read("somefile.dat");
         logTrace("Done reading file");
     } catch (Exception e) {
         logError("Failed to read input file.");
         logDiagnostic("Reported error: %s", e.msg);
         logDebug("Full exception: %s", e.toString());
	return 1;
     }
     logInfo("Input file is %d bytes", contents.length);
     logTrace("Computing sum");
     auto sum = sum(contents);
     logTrace("Removing file");
     remove("somefile.dat");
}

ulong sum(ubyte[] arr)
{
	ulong ret = 0;
	foreach (b; arr) {
		logDebugV("Adding %d", b);
		ret += b;
	}
	logDebugV("Sum result: %d", b);
	return ret;
}
---

A typical mode in my projects now is to output any level starting with 
"info" to the console by default, but allow to log "diagnostic" messages 
using "-v" (useful to the end user) and lower levels using other 
switches (useful for me to diagnose bugs).

At the same time there may be a log file or a network based remote 
logger that captures all levels regardless of command line switches, but 
needs to be able to filter based on the actual log level later in a GUI. 
Having multiple loggers for the different diagnostic/debug/trace levels 
would not really help there (except with some extensive hacking).

BTW you stated that there is a "debug" level in your implementation, but 
neither the docs, nor the pull request have it.