On Monday 15 March 2010 22:39:05 Andrey Semashev wrote:
On 03/15/2010 09:10 PM, Vladimir Prus wrote:
On Monday 15 March 2010 20:24:49 Andrey Semashev wrote:
I can't provide logger features for every case. Sometimes users will have to extend the library. Writing a class template with a couple constructors and a trivial method doesn't look very complex to me.
First, it seems to me that if two major projects use component/severity scheme, then presumably it's of general utility. Second, in the thread you have mentioned, you yourself provide example solution that are far from trivial. Maybe I'm missing something? In essence, I would like to have:
magic_logger lg; DO_MAGIC(lg, "my_component", debug)<< "hi there";
Could you show what will it take?
I've shown that, in the thread you mentioned. It simply appears that I don't think that's complex and you do. No problem, if it deems that necessary, I can add yet another feature to the library distribution.
OK.
It's not scary, it's verbose. If you want to get down to the guts of the library, you're welcome. You get the full control at the cost of verbosity. But most people won't need it and will just use the shorthands, such as "extract" and "attr". I see no problem with it.
As already expressed by Tom, logging library is fairly basic component that should be useable by everybody. And lambda functions in current C++ in not something everybody know so forcing the user to pick between using lambda, and much more verbose non-lambda style does not help. Another aspect mentioned during review is that you roll your own lambda implementation. Which means that folks that are familiar with boost::lambda might run into things that don't work, or work differently. Further, suppose one has access to a compiler that already implement native lambda. He probably would prefer that to your lambda emulation, but you impose a convenience tax on that.
Why cannot you provide and equally convenient ordinary functions?
Can you suggest a better interface? Because I can't imagine how to make it shorter, while keeping the other features intact.
Your example when using a freestanding function was: shared_ptr< logging::attribute > attr = attrs["System"]; if (attr) { optional< System > sys = attr->get< System >(); if (!sys) throw runtime_error("The System attribute has invalid type"); if (sys.get() != m_sys) return false; } else throw runtime_error("The System attribute not found"); The corresponding code using lambda would be, presumably: flt::attr< ... >("System") == m_sys Would it be possible to make the following work in the first case: if (attrs["System"] != m_sys) return false; ? Or, if changing type/behaviour of operator[] is undesirable, what about if (attrs->get_nothrow("System") != m_sys) return false; ?
* Why can't I use the library without named parameters? Surely, there are low-tech ways to specify things.
What are these?
The simplest would be:
file_log_parameters p; p.filter = ... ; p.formatter = ... ; init_log_to_file(p);
This is straightward for any user, produces clear error messages, and is not much of a inconvenience.
The filter is the property of the log frontend, and formatter is the property of the backend. Using named parameters allows to direct the appropriate parameters to their particular receivers.
Is this some magic provided by Boost.Parameters, or you still need to manuall route the properties to relevant components?
Also, it's more efficient than filling the structure.
This is init code, so performance is likely not very important.
Boost.Parameter is used in various places of the library, but its primary goal is to provide abstraction between components that would otherwise be tightly coupled. For instance, loggers consist of number of features, each of which is independent from others. Named parameters elegantly allow to interact with a particular feature without bothering other features.
Could you give more examples?
http://boost-log.sourceforge.net/libs/log/doc/html/log/extension/sources.htm...
In the end of the section you can see how two logger features are composed into a single logger. Both features require a parameter to open a log record. Named parameters allow to pass these parameters to the respective features, while neither of the features is aware of the other.
I agree that this composition of features won't work with 'big-struct-with-all-parameters' approach suggested above. Then, what about this: my_composite_logger lg; lg.set_feature_1(...); lg.set_feature_2(...); It seems to me that such interface will not require that features know about each other.
Another example. init_log_to_file may receive a number of named parameters. Some of them are used by the sink frontend, some - by the backend. In fact, you can actually pass all parameters to the frontend constructor - it will pick those it needs and pass the rest to the backend. That way the frontend is not aware of parameters that the backend requires.
Just to clarify -- is there a check that all named parameters are actually used. I admit I don't know much about Boost parameter, so might be doing something wrong, but the attached diff to basic_usage example still compiles. Is this a bug or a feature?
* The library went too far with namespaces. Why are *six* nested namespaces necessary? This appears to be just asking for extra typing for no good reason.
Use namespace aliases. Most of the time you will only need the sources namespace. Other namespaces were introduced to keep code clarity and avoid name clashes.
Of course I can use namespace aliases to alleviate the problem. However, I don't see how namespaces are necessary. Are there actually things with the same names in those different namespaces?
Yes. attr is a formatter and a filter, for example.
Oh, indeed! But.. why? Cannot one object serve both roles?
There are "format" names in "formatters" and "keywords" namespaces. Perhaps other exist, I didn't examine the code thoroughly.
Record identifiers proved to be very useful in communication, surrounding the piece of log. It's much easier to write "see, there's that error in line 2764" than to go for lengthy explanations of how to find it in the file.
What other libraries use this record id by default?
None, perhaps. That doesn't make this feature any less valuable.
Also, it prints the thread "number", on my system a hex string. I am not sure thread id should be part of trivial logging.
Why not? Why can't a multithreaded application use trivial logging?
Because hex string, in a log, is completely useless. It is of any use if you are attached to a still-running application, and can actually figure what a thread id means in your program.
I believe, Robers has already answered this. I agree with him.
Finally, I think by default, everybody things of logging as glorified std::cout, so trivial logging better print messages to console, not to a file.
I disagree. First, you don't need the library to spam the console.
Well, I do. Maybe because my console is usually not cmd.exe ;-)
You mean you can't use std::cout? What kind of console are we talking about?
Oh, of course I can use std::cout to print to console. However, I want to be able to enable and disable log message from specific components, and std::cout cannot do that.
Second, I consider spamming the console to be a bad idea in the first place. In my practice there are cases when writing _anything_ on the console is forbidden.
It seems like a fairly specialized requirement. What is trivial logging supposed to be? If it is supposed to be printf logging on steriods, then it should behave like printf as much as possible, including printing to console.
Log, in the common sense, is perceived as a file. Console is not a good place to write logs, since this is a place for user interaction, not for a protocol of execution.
I think that many non-deamon Linux program that permits enabling logging logs things to stdout. In fact, I don't remember any non-daemon program that would put logs to a file when invoked by user.
* Why does the library have light_rw_mutex? I would really prefer if everything thread-related be located inside Boost.Thread, so that at least the code can be readily audited.
light_rw_mutex can be more efficient than shared_mutex.
Can, or is?
It is, in some configurations. In particular, on POSIX and NT6 (when enabled). On POSIX someone on this list presented tests comparing pthread_rwlock_t (which is used by light_rw_mutex) against shared_mutex, former being faster. On NT6 light_rw_mutex is of size of one pointer which is ~6 times smaller than shared_mutex. The performance is comparable, according to my tests.
If it gets to Boost.Thread, I'll be first to use it.
It's not like maintainer of Boost.Thread left on a space mission to outer space and is not coming back ;-) Could you just propose this for inclusion? We have too many bits hidden in detail namespace of various libraries.
This matter is kind of separate from the library review. I have no problems proposing this component for inclusion Boost.Thread, after the review.
* It appears that the default behaviour of fmt::attr is to throw is the attribute with such name is not found. This seems like a bad choice to me. A logging library should never fail -- it is the last resort at diagnosing a problem in the field, and if it throws by default, it's useless.
That was my thought initially. Then this thread had happened:
http://article.gmane.org/gmane.comp.lib.boost.devel/185956
You can suppress exceptions with exception handlers on different levels of the library.
I saw that -- my point is that *default* behaviour should be to go on no matter what is wrong.
I think, this is probably the point where different opinions collide with none being right or wrong. In a classical case of logging as an application execution protocol writing, you're absolutely right. In other cases, when logging plays more central role in the application, the other way around is more desirable.
I'm not sure I understand those other cases. Let's wait what others will say.
* Building the library with threading=single results in unresolved references to pthread functions. I belive that library should not use any sync mechanisms in this case.
Yes, I've been told that. Presumably, the references are from Boost.ASIO, which is used by the syslog backend. Not sure what would be the best solution - to drop single-threaded builds or to exclude syslog in ST builds.
Well, if this the case, Boost.ASIO should be fixed not to contain its own thread-related mechanisms -- which only reinforces my position about rw_mutex.
No, it doesn't reinforce your point. ASIO may well not be suited to be used in a ST environment.
It might not be suited, but by using custom threading code as opposed to Boost.Thread it manifests this unsuitability in a inconvenient way. - Volodya
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Thanks,