Skip to content

Conversation

annagrin
Copy link
Contributor

@annagrin annagrin commented Feb 7, 2023

List instances received from getObject contain one extra element (comes from JavaScript Array.length property). Make sure we don't count the length field in the returned list instance.

@annagrin annagrin requested a review from elliette February 7, 2023 23:26
@annagrin annagrin requested a review from elliette February 9, 2023 00:18
/// Ignore any non-elements like 'length', 'fixed$length', etc.
static List<Property> _listElementProperties(List<Property> properties) =>
properties
.where((p) => p.name != null && int.tryParse(p.name!) != null)
Copy link
Contributor

@elliette elliette Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the lamda can be simplified to (p) => int.tryParse(p.name ?? '') != null because int.parse('') returns null.

Copy link
Contributor Author

@annagrin annagrin Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original is faster though, so prefer to leave it as is for now.

}
/// Return only elements of the list from [properties].
/// Ignore any non-elements like 'length', 'fixed$length', etc.
static List<Property> _listElementProperties(List<Property> properties) =>
Copy link
Contributor

@elliette elliette Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOC, do you know why this is static?

Copy link
Contributor Author

@annagrin annagrin Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not need to access any methods or fields. Normally it is much faster to call static methods as they don't require a virtual table but I am not sure it is true for dart. There might be a lint for it as well, I'll try to find it.

@annagrin annagrin merged commit f1b15e8 into dart-lang:master Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants