On 07/03/2016 02:21 PM, Guillaume Munch wrote:
> Le 27/06/2016 23:15, Richard Heck a écrit :
>> On 06/27/2016 04:14 PM, Georg Baum wrote:
>>> You mean std::bind, or did I miss any remaining boost::bind?
>
> I just removed some leftover boost::function (not boost::bind).
>
>>> For me personally the patch is too big to proof-read it. I'd prefer to
>>> change simple things like
>>>
>>> - BufferStorage::iterator it = bstore.begin();
>>> - BufferStorage::iterator end = bstore.end();
>>> - for (; it != end; ++it) {
>>> - Buffer * buf = *it;
>>> + for(Buffer * buf : bstore)
>>>
>>> which are not related to removal of std::bind first.
>>
>> Yes, that's a good idea.
>
>
> I have split the patch in two for you, and already committed the part
> that does not introduce lambda expressions. Attached is the remainder of
> the patch that replaces all remaining std::bind with lambda expressions.
>
>
>>
>>> Apart from that I am not too familiar with lambdas yet, so for me
>>> personally
>>> the new code is more difficult to read than the old one ATM, and I am
>>> probably not the only one. Your explanation to Richard is nice, but
>>> it will
>>> soon be buried in email archives, and people reading LyX sources do
>>> not see
>>> it.
>>>
>>> I am not opposed to using lambdas (I hope I can learn soemthing from
>>> you
>>> here), but I believe that our developer documentation should contain
>>> a short
>>> motivation why they are used in LyX (you basically explained already to
>>> Richard how they make code more readable), and a pointer to a more
>>> detailed
>>> introduction to lambdas. Then it should also be possible for novices
>>> or old-
>>> timers like me to understand the new hot stuff.
>>
>> It wouldn't hurt to put some comments in the code, too.
>>
>
> I am happy to explain my code to anybody who asks. However, I am not
> going to introduce a motivation for lambda expressions in the developer
> manual nor add links to introductions. The old code was already using
> anonymous functions; that lambda expressions is better for anonymous
> functions is obvious to anybody familiar with lambda expressions; and
> the documentation is not meant to be a tutorial for ISO C++.
My own view is that our code is undercommented---and I know I'm as bad
about this as anyone. That said, I often add comments to code I have to read
to fix various bugs, if only for my own benefit later.
You're right that it's clear enough what the intent is in many cases.
But I don't
think that's always true, and it wastes more time to decode something
than it
would take to comment it. Generally, cases like:
[this]() { someFunction(); }
are obviously easy to read. It's more complicated when there are various
arguments. One might wonder in such cases why things are being copied
or passed by reference, for example.
> Neither am I going to add comments to the code affected by the patch,
> because nobody felt like requiring such comments for the old code.
> Whereas the std::bind version can be hard to read for anybody, the new
> code is self-explanatory for anybody familiar with lambda expressions,
> and comments are again not meant to explain the language.
No doubt the std::bind versions are write-only, but that only illustrates
my point: Our code is undercommented. In this case, for example:
@@ -741,7 +740,9 @@ void PreviewLoader::Impl::startLoading(bool wait)
// Initiate the conversion from LaTeX to bitmap images files.
ForkedCall::SignalTypePtr
convert_ptr(new ForkedCall::SignalType);
- convert_ptr->connect(bind(&Impl::finishedGenerating, this, _1, _2));
+ convert_ptr->connect([this](pid_t p, int retval) {
+ finishedGenerating(p, retval);
+ });
ForkedCall call(buffer_.filePath());
int ret = call.startScript(command, convert_ptr);
I'm not sure that the original author's decision not to add a comment is
a reason we shouldn't add one.
> If other developers are not familiar with lambda expressions yet, then
> it is appropriate to wait before committing. Given that I do not know
> the source of my issue with gcc 4.6, it is appropriate to wait that
> support for gcc 4.6 is dropped (probably about a year from now).
I don't have a problem with the patch's being committed. But it's worth
remembering that most of us are amateurs. We don't all keep up with the
latest developments in compiler technology. As a result, discussions
about ways to make our codebase accessible to amateurs aren't at all
uncommon. I knew almost no C++ when I started working on LyX. The
way I learned, and especially the way I learned STL, was by doing.
What was the issue with gcc 4.6?
Richard
