On 8/23/2013 9:48 AM, Robert Schadek wrote: > On 08/23/2013 02:56 PM, Piotr Szturmaj wrote: >> W dniu 23.08.2013 14:16, Robert Schadek pisze: >>> On 08/23/2013 01:10 PM, Piotr Szturmaj wrote: >>>> so if I have a LogLevel variable I want to use: >>>> >>>> LogLevel myLogLevel; >>>> >>>> I need to write this: >>>> >>>> # alternative 1 (possibly wrapped in a function) >>>> switch (myLogLevel) >>>> { >>>> case LogLevel.Info: log.Info("..."); break; >>>> case LogLevel.Warning: log.Warning("..."); break; >>>> case LogLevel.Error: log.Error("..."); break; >>>> case LogLevel.Critical: log.Critical("..."); break; >>>> case LogLevel.Fatal: log.Fatal("..."); break; >>>> } >>>> >>>> #alternative 2 >>>> log.logLevel = myLogLevel; // not thread-safe (shared state) >>>> log("..."); >>> the default logger is not shared. so no thread problems. >> >> But the problems remain with non-default loggers? > I don't see your point. Currently there are two trivial logger. An stdio > and a file logger. Both are thread local. > There are only these two, because I believe that you cannot create a > logging library that fulfills everybody requirements and > trying will lead to a logger library nobody can use anymore. So I took > the opposite direction and made it simple merely defining > a common interface to make logging to defined types of loggers uniform. > > So if you need a logger that is thread safe. Please created on and use it. >> >>>> >>>> I mean sure, it is possible to specify LogLevel for each message, but >>>> only at compile time (by the use of function names .Info, .Error, >>>> etc.). >>>> >>>> This is not what I meant in my previous post. I rather meant the >>>> ability to specify LogLevel using a runtime variable. >>> >>> auto myLogger = new MyLogger(myLogLevel); >>> mylogger("my log msg"); >> >> So, we're at the start point. You must either create instances for >> every log level (and for every logger class) or set a log level before >> calling the log() - and this is not thread safe for custom loggers. >> >> This also doesn't look as a good design: >> >> auto myLogger = new MyLogger(myLogLevel); >> myLogger("my log msg"); >> myLogger.logLevel = anotherMyLogLevel; >> myLogger("another log msg"); >> myLogger.logLevel = yetAnotherMyLogLevel; >> myLogger("yet another log msg"); > If the LogLevel you are using are not const and are computed during the > lifetime of the program. Than yes, that is the way you have to use it. > But if anotherMyLogLevel and yetAnotherMyLogLevel are mere const, why > not write myLogger.warning("...."); myLogger.error("...")? > >> Why encapsulate a logLevel which is immediately used in the function? >> Why store it in a class? It should be a function parameter instead. > I'm not sure if I understand your first sentence! Having a LogLevel > stored in a logger allows to disable the logging of some of the messages > send to > a specific logger. > > Example: > auto myLogger = new Logger(); > myLogger.warning("Something is fishy"); > myLogger.warning("Something else is fishy"); > > next round I want to disable warning messages logged by myLogger. I > don't want to touch all the .warning("..") lines. I want to write > myLogger.logLevel = LogLevel.Fatal; and disable all warning messages > this way. > > >> >> Don't get me wrong. I just want to help you. >> >> I think that default logger concept should be separated to default >> logger and default log level (default log level should be independent >> from default logger) > It is separated! LogManager.globalLogLevel = LogLevel.XXXXX; sets the > global LogLevel. The defaultLogger is just another logger which can be > found at a special > place.
Piotr brings up good very points and I would say your answers don't fully address them. Why should there be 5 differently named methods for something that does *the same thing* except for logging level? What is the advantage over having just one with level passed as a parameter? Also, if I understand it correctly, the class has a method that will log *the same message* at different levels based on some centralized configuration. That doesn't sound like a good idea. Severity of an error depends on what it is. It doesn't sound right to change it based on some centralized config setting in an unrelated class. I.e. if something is a fatal error, it should always be explicitly, obviously fatal. Changing that *should* require an explicit change at the place where the error is logged. Frankly, I would not allow changing loggers LogLevel after initialization either. There is simply no good reason to do that and sound so would lead to really annoying issues when analyzing code. (See PHP's awful @ operator.) Lastly, I think what Piotr meant by "separated" is that there is a common need to separate log *collector* (which should be as standard as possible) and log *emitters* (which would do different things for the same message, like sending emails, writing to files, outputting the yellow screen of death, etc). LogLevel should be a property of log emitters. That's a very, very common need. You want to write every single message to a file, but you only want to get fatal errors via email.