On Sun, Sep 2, 2012 at 8:47 AM, Pavel Sanda <sa...@lyx.org> wrote:
> Scott Kostyshak wrote:
>> @@ -2453,7 +2453,7 @@ void GuiApplication::hideDialogs(string const & name, 
>> Inset * inset) const
>>  Buffer const * GuiApplication::updateInset(Inset const * inset) const
>>  {
>>       Buffer const * buffer_ = 0;
>> -     QHash<int, GuiView *>::iterator end = d->views_.end();
>> +     QHash<int, GuiView *>::const_iterator end = d->views_.end();
>>       for (QHash<int, GuiView *>::iterator it = d->views_.begin(); it != 
>> end; ++it) {
>>               if (Buffer const * ptr = (*it)->updateInset(inset))
>>                       buffer_ = ptr;
>
> I wonder what is the gain in cases like this.

I would actually prefer changing iterator 'end' to 'const_iterator const end'.
And I would also suggest changing
'QHash<int, GuiView *>::iterator it' to 'QHash<int,
GuiView*>::const_iterator it'

I don't have a strong opinion either way, but just to play devil's advocate
(and I do believe in the following to a small extent):

(1) There is the following line around line 1317 in the same file: QHash<int,
GuiView *>::const_iterator end = d->views_.end(); If I see this line and then I
see a similar line but that one thing is different I'm going to think either
(a) the style is inconsistent
or
(b) the difference is for a reason. I will ask myself "why use const_iterator in
one place of the code and not use const_iterator in another place?" to
which I might answer "they must serve different functions".

(2) It doesn't seem too contrived to have code with a loop that
iterates through the
elements and then does something to the last element. Using a const_iterator
instead of a normal iterator tells me "I'm not going to change the last
element".

(3) I've read that compilers can apply more aggressive optimizations when more
of the data is const.  I'm not sure if this would apply here and even if it did
the gain would clearly be negligible, but I figure that in no case could
const_iterator perform worse.

Any thoughts?

Thanks for looking at the patch and for bringing this topic up. I hope
I'm not the
only one that enjoys this type of discussion.

Scott

Reply via email to