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

Subject: Re: [boost] [log] Review-ready version in the Vault
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2009-02-12 13:32:59

----- Original Message ----- 
From: "Andrey Semashev" <andrey.semashev_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Thursday, February 12, 2009 7:12 PM
Subject: Re: [boost] [log] Review-ready version in the Vault

> 
> 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?

Why not making the filter just in a view of attributes values, not the record itself. If I have understood, the library do not proposse firter on log records.

>>> 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.

Thanks :)
 
>>> [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.

Sorry, I don't undesrtand. Could you elaborate on that.
 
>>>> 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 was not talking of \n record separator, but std::endl which is a \n + flush. In My opinion, is the flush operation which should pust the record.  
Well you can make a wrapper with the IoStream library, and should be able to redefine the flush operation.

> I think that scoped records are the way to go.

Maybe.
 
>>> 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.

The BOOST_LOG macro is another issue. When you use the BOOST_LOG  you can state this macro push a log record, and maybe you will need to use the current interface. My concern is that the application must be able to master the log record delimiter, not that this must be done on the macro.

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

It will be clearer.

Thanks for all your answers,
Vicente