Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(975)

Unified Diff: pkg/serialization/lib/src/basic_rule.dart

Issue 12091026: Serialization shouldn't modify the keys of a map it's iterating over (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Created 7 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: pkg/serialization/lib/src/basic_rule.dart
diff --git a/pkg/serialization/lib/src/basic_rule.dart b/pkg/serialization/lib/src/basic_rule.dart
index a22ea0a2841be1885309df0d81f2ade9fed5ed97..c989b2bdd3f189f0fba4c164e875107f655aa2fb 100644
--- a/pkg/serialization/lib/src/basic_rule.dart
+++ b/pkg/serialization/lib/src/basic_rule.dart
@@ -142,21 +142,20 @@ class BasicRule extends SerializationRule {
* resolve them to the strings we expect. We leave the previous keys in there
* as well, as they shouldn't be harmful, and it costs more to remove them.
*/
- makeIndexableByNumber(state) {
- if (!(state is Map)) return state;
- // TODO(alanknight): This is quite inefficient, and we do it twice per
- // instance. If the keys are references, we need to turn them into strings
- // before we can look at indexing them by field position. It's also eager,
- // but we know our keys are always primitives, so we don't have to worry
- // about their instances not having been created yet.
- for (var each in state.keys) {
- if (each is Reference) {
- var inflated = each.inflated();
- state[inflated] = state[each];
- }
- }
- return new _MapWrapper.fromMap(state, fields.contents);
- }
+ makeIndexableByNumber(state) {
+ if (!(state is Map)) return state;
+ // TODO(alanknight): This is quite inefficient, and we do it twice per
+ // instance. If the keys are references, we need to turn them into strings
+ // before we can look at indexing them by field position. It's also eager,
+ // but we know our keys are always primitives, so we don't have to worry
+ // about their instances not having been created yet.
+ var newState = new Map();
Jennifer Messerly 2013/01/28 19:45:00 one idea that might help, if the perf here is noti
+ for (var each in state.keys) {
+ var newKey = (each is Reference) ? each.inflated() : each;
+ newState[newKey] = state[each];
+ }
+ return new _MapWrapper.fromMap(newState, fields.contents);
+ }
/**
* Extract the state from [object] using an instanceMirror and the field
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698