Re: [boost] [log] Review-ready version in the Vault

Subject: Re: [boost] [log] Review-ready version in the Vault
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2009-02-12 13:12:09

vicente.botet wrote:

>>> Unacceptable IMHO. The user don't need to check if logs are enabled. 
>> He does need to open a record in some way. Opening a record is not only 
>> filtering, although this is a part of the process. The key point of 
>> opening a record is acquiring values of all attributes and composing 
>> them into a single view. It is this view then processed by filters, 
>> formatters and sinks.
> 
> I have no problem with opening the log record, the problem is that you close the record as soon as the destructor on the remporary is called.
> 
> Anyway, this is an implementation detail. Checking should be be able to be done without a log record opened.
> 
>>> Why not have 3 functions enabled, open_record and close_record, and why not have a scoped_record?
>> For the reason I've described above. "enabled" will have to implicitly 
>> call "open_record".
> 
> I desagree in this pooint.

You cannot execute the filter without having a view of attribute values 
because the filter makes its decision based on this view. Do you have 
something different on your mind?

>> On the other hand, I realize that the current interface is 
>> not perfect, too, so I have no strong position. FWIW, it should be quite 
>> simple to change this.
> 
> I'm sure that this will not be complicated. But this can not be made from outside, so I need you change it.

Ok, I'll do so.

>> [snip]
>>
>> All these cases are covered by stream manipulators and operator<< 
>> overriding. I must say, this approach is much more appealing than 
>> writing inline loops etc., since it may be reused regardless of logging.
> 
> This was just an example. You will need to inline loops on the  operator<< overriding, isn't it?

Yes, but these manipulators or operator<< overloads can be reused.

>>> What do you think about this 
>>> if (lg.enabled()) {
>>>     for (it=begin;it!end; ++it) { 
>>>         lg << *it ;
>>>         if (it+1!=end) lg << ", ";
>>>         else lg << std::endl;
>>>     }
>>> }
>>>
>>> std::endl will close the record.
>> I would really dislike to override behavior of the standard 
>> manipulators. 
> 
> Why? Is there a deep reason?

The left-hand argument of operator<< is an std::basic_ostream, which 
handles these manipulators itself. So, the only way to detect the 
manipulator in action is to observe its effects in the stream buffer. In 
case of std::endl this would mean searching the formatted data for '\n', 
which is a performance loss and is not valid in general (since a log 
record can contain several lines of text).

I think that scoped records are the way to go.

>> That looks much more attractive. However, I had something like this in mind:
>>
>>   if (log_record rec = lg.open_record())
>>   {
>>     rec.strm() << ...;
>>   }
>>
>> The one problem I see here is how to deduce the log_record type from lg. 
>> I don't want to use Boost.TypeOf since it looks too heavy to me.
> 
> What about defining it inside the logger?
> 
>     logger_type lg;
>     if (logger_type::scoped_log_record rec = lg.open_record())
>     {
>         rec.strm() << ...;
>     }

The problem is that I don't have logger_type in the BOOST_LOG(lg) macro.

> Why do you need to get the strm to be able to log? Why not just
> 
>         rec << ...;

I can do that.