views: STL iterator cleanups in View class.

views: STL iterator cleanups in View class.

- Standardize iterator naming to |i|.
- Initialize them through constructor not assignment operator.
- Declare iterators inside loops, not above them.

BUG=72040
TEST=None

R=pkasting@chromium.org,sky@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88551

[Peter Kasting, 2011-06-09 00:43:32]

LGTM, I guess

http://codereview.chromium.org/7129028/diff/1002/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/7129028/diff/1002/views/view.cc#newcode876
views/view.cc:876: std::vector<Accelerator>::iterator
i(std::find(accelerators_->begin(),
Nit: Linebreak the way the old code did (after the '(', nowhere else).

http://codereview.chromium.org/7129028/diff/1002/views/view.cc#newcode887
views/view.cc:887: std::vector<Accelerator>::iterator
i(std::find(accelerators_->begin(),
This isn't safe; you moved use of |accelerators_| above its NULL-check.

http://codereview.chromium.org/7129028/diff/1002/views/view.cc#newcode1540
views/view.cc:1540: view));
Nit: I'd combine lines 2 and 3

[tfarina, 2011-06-09 00:57:00]

http://codereview.chromium.org/7129028/diff/1002/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/7129028/diff/1002/views/view.cc#newcode876
views/view.cc:876: std::vector<Accelerator>::iterator
i(std::find(accelerators_->begin(),
On 2011/06/09 00:43:33, Peter Kasting wrote:
> Nit: Linebreak the way the old code did (after the '(', nowhere else).

Done.

http://codereview.chromium.org/7129028/diff/1002/views/view.cc#newcode887
views/view.cc:887: std::vector<Accelerator>::iterator
i(std::find(accelerators_->begin(),
On 2011/06/09 00:43:33, Peter Kasting wrote:
> This isn't safe; you moved use of |accelerators_| above its NULL-check.

Done.

[Peter Kasting, 2011-06-09 00:59:38]

http://codereview.chromium.org/7129028/diff/3/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/7129028/diff/3/views/view.cc#newcode886
views/view.cc:886: if (!accelerators_.get())
This doesn't NOTREACHED() the way the old code does.

[tfarina, 2011-06-09 01:08:17]

http://codereview.chromium.org/7129028/diff/3/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/7129028/diff/3/views/view.cc#newcode886
views/view.cc:886: if (!accelerators_.get())
On 2011/06/09 00:59:38, Peter Kasting wrote:
> This doesn't NOTREACHED() the way the old code does.

Done.

[tfarina, 2011-06-09 17:04:07]

On 2011/06/09 00:43:32, Peter Kasting wrote:
> LGTM, I guess
> 
sky, can I get your owner's lgtm as well?

[sky, 2011-06-09 17:06:34]

LGTM

http://codereview.chromium.org/7129028/diff/5002/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/7129028/diff/5002/views/view.cc#newcode878
views/view.cc:878: DCHECK(i == accelerators_->end())
move the std::find into the DCHECK, eg:
DCHECK(std::find(...) == accelerators_->end())

[tfarina, 2011-06-09 17:17:06]

http://codereview.chromium.org/7129028/diff/5002/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/7129028/diff/5002/views/view.cc#newcode878
views/view.cc:878: DCHECK(i == accelerators_->end())
On 2011/06/09 17:06:34, sky wrote:
> move the std::find into the DCHECK, eg:
> DCHECK(std::find(...) == accelerators_->end())

Done.