Re: [boost] [log] Comments

Subject: Re: [boost] [log] Comments
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2010-03-15 18:27:20

On 03/15/2010 11:22 PM, Vladimir Prus wrote:
>
>>>> 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;

It doesn't specify the type of the value.

> ? Or, if changing type/behaviour of operator[] is undesirable, what
> about
>
> 	if (attrs->get_nothrow("System") != m_sys)
> 		return false;

That should be:

   if (attrs.get_nothrow< System >("System") != m_sys)
     return false;

Yes, it could be done. But as stated elsewhere in the discussion, this 
would require System to be copyable (which might be fine in this case). 
And it doesn't really differ from:

   bool result = false;
   extract< System >("System", attrs, var(result) = _1 != m_sys);
   return result;

but with extract it doesn't have the copyability restriction.

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

No, I just pass on the whole pack of arguments.

>> Also, it's more
>> efficient than filling the structure.
>
> This is init code, so performance is likely not very important.

open_record is called for every log record.

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

It will not, unless they clash member functions, but we can ignore that 
for now. But it doesn't help with constructors, and makes writing macros 
around this interface a bit harder.

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

Unused parameters are ignored, no compile-time or run-time errors. I 
can't call that a feature, but it certainly is not a bug. Just an 
expected behavior by design.

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

One is filter and the other is formatter. They have different interfaces 
(with regard to the library and the user) and serve different purposes 
(one - to compose filters, the other - formatters). Also, thread safety 
requirements are different. So I don't think that mixing them is a good 
idea. And still, having the same name makes sense, because 
syntactically, both in filters and formatters these placeholders 
represent an attribute value.