Bug 70285 – Style guide should mention the preference of index over iterator

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Website

[Ryosuke Niwa]

Per discussion on webkit-dev, we should mention that we prefer using index over iterator to go though items in a Vector.

[Ryosuke Niwa]

https://lists.webkit.org/pipermail/webkit-dev/2011-October/018274.html

[Ryosuke Niwa]

Created attachment 111351 [details]
70285

[Ryosuke Niwa]

Any reviewer?

[Darin Adler]

Comment on attachment 111351 [details]
70285

For the index case we chose to evaluate size every time through the loop. For the iterator case we chose to cache the end iterator. That’s an arbitrary difference not related to index vs. iterator. I think a local variable named size would be a better way to do things.

I don’t think this makes it clear enough that there are data structures where iterators are present, but indexing is not. Nor does it answer the question of why we offer iterators if indexing are preferred.

Also, this doesn’t contain any of the rationale; perhaps it should.

[Ryosuke Niwa]

Created attachment 111690 [details]
Updated per comment

[Ryosuke Niwa]

Ping reviewers again

[Darin Adler]

Comment on attachment 111690 [details]
Updated per comment

View in context: https://bugs.webkit.org/attachment.cgi?id=111690&action=review

> Websites/webkit.org/coding/coding-style.html:712
> +<li>Prefer index over iterator in Vector iterations for a terse, easier-to-read code, and to avoid accessing freed memory

I don’t see the end of this <li> element.

It’s overselling this technique to say that it avoids accessing freed memory, because using a too high index can still access freed memory.

> Websites/webkit.org/coding/coding-style.html:717
> +size_t frameViewSize = frameViews.size();

frameViewSize is not the right name here. Maybe frameViewsSize or frameViewsCount or size or count.

[Ryosuke Niwa]

(In reply to comment #7)
> (From update of attachment 111690 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111690&action=review
> 
> > Websites/webkit.org/coding/coding-style.html:712
> > +<li>Prefer index over iterator in Vector iterations for a terse, easier-to-read code, and to avoid accessing freed memory
> 
> I don’t see the end of this <li> element.

It's the pattern used throughout the file. I should probably correct that once for all.

> It’s overselling this technique to say that it avoids accessing freed memory, because using a too high index can still access freed memory.

Okay, I'll remove that.

> > Websites/webkit.org/coding/coding-style.html:717
> > +size_t frameViewSize = frameViews.size();
> 
> frameViewSize is not the right name here. Maybe frameViewsSize or frameViewsCount or size or count.

Oops, typo. It was supposed to be frameViewsSize but I guess frameViewsCount sounds better. Will change before landing.

[Ryosuke Niwa]

Committed r98166: <http://trac.webkit.org/changeset/98166>