Christian Ridderström <[EMAIL PROTECTED]> writes:
| On Mon, 5 Jan 2004, Lars Gullik Bjønnes wrote:
>
>> If you want obfuscation have a look at this.
>> (no, I won't actually admit that this is obfuscated.)
>> (I find especially the logical_and usage a bit funny :-) )
>
| Ok... I read about half of it before I got tired... My conclusion is
| that Lars is essentially correct;
That was a first :-)
| I was surprised how easy it was to
| understand it in general now.
If you as a not so experienced C++ STL programmer can grasp the use of
bind this quickly I am not worried about obfuscation.
| However, a few general comments
| (although perhaps more about C++ in general than this particular
| code) ...
>
| Look at this code:
[...]
| Should the code readability really depend _this_ much on the
| indenting?
Perhaps not.
| Maybe the code is not obfuscated, but the statements are definitely
| more complicated now... compare this to (I still don't know how to write
| this code, so ???? stands for some declaration of a bound object):
>
| boost::function<bool(Converter const & c)>
| compare_Converter(string const & from, string const & to)
| {
| bind????? fromConvertMatch_p =
| bind(equal_to<string>(), bind(&Converter::from, _1), from);
| bind????? toConvertMatch_p =
| bind(equal_to<string>(), bind(&Converter::to, _1), to));
| return bind(logical_and<bool>(), fromConvertMatch_p, toConvertMatchP);
| }
boost::function<bool(Converter const & c)>
compare_Converter(string const & from , string const & to)
{
boost::function<bool(string const &, string const &)> eq_from =
bind(equal_to<string>(),
bind(&Converter::from, _1, from));
boost::function<bool(string const &, string const &)> eq_to =
bind(equal_to<string>(),
bind(&Converter::to, _1, to));
return bind(logical_and<bool>(), eq_from, eq_to);
}
Might work. I have not tested that.
| If we wanted more compact statements, couldn't we rewrite
| for ( ; par != end; ++par) {
| if (par.pit() == pit)
| break;
| }
>
| into something like this
| while ( par != end && !(par.pit() == pit) )
| ++par;
>
| but I think the first form is easier to read.
No I don't agree. The loop above is about finding something so IMHO
that should be explicitly mentioned:
iterator par = find(beg, end, FUNCTOR);
Either the FUNCTOR should have a nice name so that it is easly
understandable what we are finding, or the buildup of the FUNCTOR
should be explicit in the call.
| Anyway, the function compare_Converter() was still readable thanks
| to the indentation, but piece of code from
| LyXTextClass:LyXTextClass() is over the top (note that I changed the
| indentation to keep one of the lines from wrapping):
>
| return find_if(layoutlist_.begin(), layoutlist_.end(),
| bind(equal_to<string>(),
| bind(&LyXLayout::name,
| bind(&LyXLayout_ptr::operator->, _1)),
| name))
| != layoutlist_.end();
>
| actually... I'm not sure I've got the indentation correct here...
this is because we stroe LyXLayout_ptr in the layoutlist_.
LyXLayout_ptr is a shared_ptr and we must dereference with operator->
before we can call member functions on the under lying LyXLayout.
| Finally, the lambda thing looks cool... while trying to find out
| about bind(...), I found this link:
| http://www.boost.org/libs/lambda/doc/ar01s03.html
>
| and based on that, it looks like compare_Converter() could be written as:
>
| bool BranchList::remove(string const & s)
| {
| List::size_type const size = list.size();
>
| // list.remove_if(var(s) == bind(&Branch::getBranch, _1)); // a)
>
| // list.remove_if(s == bind(&Branch::getBranch, _1)); // b)
>
| list.remove_if(s == _1.branch_); // c)
>
| // list.remove_if(s == getBranch(_1)); // d)
>
| return size != list.size();
| }
>
| I'm not sure if alt. (a) works, and although I like alt. (b), it doesn't
| use the getBranch() method. In that case, I think alt (d) should
| work.
I think (s == _1.getBranch()) should work. Or some close relative of
that.
PS: Thanks a lot for you input. Just what I wanted.
--
Lgb
