Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement .iter to DOMRectList #11162

Closed
wants to merge 1 commit into from

Conversation

@jaysonsantos
Copy link

jaysonsantos commented May 12, 2016

This will be used in #10828 but I made thins in a splited PR because the other one is pretty hard to get working and this iterator still would be useful.


This change is Reviewable

@@ -57,3 +66,18 @@ impl DOMRectListMethods for DOMRectList {
self.Item(index)
}
}

impl<'a> Iterator for Iter<'a> {

This comment has been minimized.

@KiChjang

KiChjang May 12, 2016

Member

Why don't you implement Iterator directly for DOMRectList instead?

This comment has been minimized.

@cbrewster

cbrewster May 12, 2016

Member

Iter stores the current position, but perhaps it would be better to rename Iter to IterableDOMRectList or DOMRectListIterator

This comment has been minimized.

@jaysonsantos

jaysonsantos May 13, 2016

Author

@KiChjang I though that but it would mutate the DOMRectList and it would only be iterated once because of position. Does it make sense?
@ConnorGBrewster I used the same naming as in the std, is better to change it?

This comment has been minimized.

@KiChjang

KiChjang May 13, 2016

Member

Actually, the better question here is why not simply return self.rects.iter() in fn iter.

This comment has been minimized.

@jaysonsantos

jaysonsantos May 13, 2016

Author

I am maitaining Item signature which is Root<DOMRect> and self.rects is a vector of JS<DOMRect>

This comment has been minimized.

@KiChjang

KiChjang May 13, 2016

Member

self.rects.iter().map(|rect| Root::from_ref(&*rect))?

This comment has been minimized.

@Ms2ger

Ms2ger May 13, 2016

Contributor

You can't return that from a function. However, it would probably be better to store self.rects.iter() in a field of Iter and then return self.inner.next().map(|rect| Root::from_ref(&*rect)) from next().

@nox
Copy link
Member

nox commented May 13, 2016

I don't understand, why do we need a specific iterator instead of just returning a &[&DOMRect]?

@jaysonsantos
Copy link
Author

jaysonsantos commented May 13, 2016

@nox I don't know either :) I am just starting at this business. But to return it I would have to iterate over all rects to make it, no? And it wouldn't be lazy.

@KiChjang
Copy link
Member

KiChjang commented May 13, 2016

Superseded by #11171.

@KiChjang KiChjang closed this May 13, 2016
@jaysonsantos jaysonsantos deleted the jaysonsantos:iterator-domrectlist branch May 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.