Re: [boost] [Logging] Documentation comments.

From: Jurko Gospodnetić (jurko.gospodnetic_at_[hidden])
Date: 2008-02-11 04:56:56

   Hi John.

>>    * Some nitpicking.
>>      * Many lists inconsistently have items with and some without a 
>> trailing dot.
>>      * Many lists inconsistently have lowercase/uppercase leading letters.
>>   
> this took a while to fix...

   Hehe... this was the one item I actually expected to be brushed off 
with a 'what the f***'-style comment... Hats off for the effort... :-)))

>>        * If the user does not call this function explicitly, when will 
>> it get called? And why do you keep calling it explicitly in all the 
>> example code?
>>   
> it needs to always be called explicitly
>>        * Seems like a bad name for what it is intended to do. If it 
>> actually does change the logger's state to 'initialized' then it should 
>> be an internal implementation detail function and not made public. If it 
>> only raises a flag meaning 'do not cache log messages for this logger' 
>> then it should be renamed to something like disable_caching() or 
>> enable_caching( false ).
>>   
> It actually gets all messages that were cached, and writes them to their 
> logger destinations.

   Perhaps there is a different way to implement this that would prevent 
errors such as forgetting to 'mark a logger as initialized' and also not 
cause the developer to ask himself 'why is this library so complicated, 
why do I need to call something called 'mark_as_initialized'...

   Some ideas (note that these are just things 'off the top of my head' 
so might not be represent a good design as they stand :-)):

     - better naming... documenting that a logger may be in two states - 
enabled and disabled, that caching is used while the logger is in the 
disabled state and renaming mark_as_initialized() to 
enable()/set_enabled()/set_state()/something to that effect...

     - effectively two states but implemented by allowing the user to 
prepare some sort of a LoggerParameters object that would contain all 
the Logger settings he needs and then passing that LoggerParameters 
object to a Logger constructor. Then constructing a Logger in such a way 
would automatically be in the enabled state and you can still have a 
separate constructor that enables caching if needed. That way someone 
who does not care about multiple-threads would not have to even consider 
things like 'my logger has not been initialized yet' (and this does seem 
as the most common use case) and those that do would have an 
understandable interface for making sure their separate threads get 
synchronized correctly.

     - another variation (and the one that seems like the best choice) 
would be to make the logger active by default and only disable it (i.e. 
enable caching) explicitly. That way the most common use case would be 
simple and intuitive... and when someone needs caching - they will have 
no objection to having to enable it - and will even be glad that they 
have that functionality available. :-)

>>      * How do BOOST_LOG_BEFORE_INIT_LOG_ALL, 
>> BOOST_LOG_BEFORE_INIT_CACHE_FILTER and 
>> BOOST_LOG_BEFORE_INIT_IGNORE_BEFORE_INIT macros work together? Since 
>> BOOST_LOG_BEFORE_INIT_LOG_ALL is on by default do the other two simply 
>> override this one? Or is the user expected to disable this one somehow?
>>
>>   
> The other two can override it.

   If you implement 'optional' caching as suggested above then would not 
all these macros be made redundant? This would also be a step towards 
pleasing all the 'macro-are-evil' people... :-) (Note, I am one of 
them... just have not still taken the time to fully understand why each 
of them is used in Boost Logging. :-) )

> fixed
> fixed
> fixed
> fixed
> fixed

  Thanks... :-)

>>    * How would one achieve the following: you have an application 
>> logging messages organized into different groups 
>> (categories/whatever...) and you want to say at run-time: log this group 
>> to this file and to debug output, do not log that group at all and log 
>> that third group only to this file. Would this require separate logger 
>> objects? Or can filters somehow decide which destinations they 
>> disable/enable?
>>   
> You'd need a different logger per category.

   Hmmm... ok... I have not taked the time to research this... but my 
fear with this is that this might cause logging support to have a large 
footprint.

   This also seems to be related to one of the objections made by Andrey 
Semashev - with supporting more detailed filtering criteria. His context 
entities would seem to correspond to what I named 
'categories/whatever...' above.

>>    * What are the exception safety guarantees? What can throw 
>> std::bad_alloc for example? What other exceptions can be thrown by the 
>>   
> There are a few objects that can throw std::bad_alloc - the classes that 
> use TSS and array::shared_ptr_holder.
> I can pre-allocate some memory for the shared_ptr_holder and it will be 
> a bit harder for the TSS classes.

   Making it not throw is one thing - useful but perhaps not strictly 
necessary. In any case could you just document which functions may throw 
what (possibly stating where needed that they throw whatever gets thrown 
by some user callback - if such things are used).

>>    * http://torjo.com/log2/doc/html/namespaceboost_1_1logging_1_1tag.html
>>      * What is that '#include <boost/logging/format_fwd.hpp>' line in 
>> the 'Tags - explained' part.
>>   
> what you need to include in order to use tags

   Perhaps add that note to the docs. :-).

> Note: I'll upload the changes to http://torjo.com/log2/doc/v3/html/

   The link does not work for me. Perhaps you just did find the time to 
upload it yet.

   Best regards,
     Jurko Gospodnetić