Skip to content

Commit

Permalink
unreads [nfc]: Use for/in style instead of forEach
Browse files Browse the repository at this point in the history
Sometimes we get feedback from a lint rule --
  https://dart.dev/tools/linter-rules/avoid_function_literals_in_foreach_calls
-- whose documentation recommends using for loops instead of
forEach.

That rule didn't fire on these instances; shrug; but this file has
had a mix of for loops and forEach loops, and it seems nicer to
consistently use one or the other.

Just in case there was a performance bug in a Map.forEach
implementation, I tested mark-as-read (by scrolling through the web
app) to see if we might get a free performance benefit there, where
the norm on my device (with a profile build) has been to take about
14ms per event:
  zulip#304 (comment)
But, unsurprisingly, it stayed at around 14ms as before.
  • Loading branch information
chrisbobbe committed Oct 26, 2023
1 parent a7f7fff commit cc76f2a
Showing 1 changed file with 21 additions and 12 deletions.
33 changes: 21 additions & 12 deletions lib/model/unreads.dart
Original file line number Diff line number Diff line change
Expand Up @@ -287,14 +287,23 @@ class Unreads extends ChangeNotifier {
.add(messageId);
}
}
newlyUnreadInStreams.forEach((incomingStreamId, incomingTopics) {
incomingTopics.forEach((incomingTopic, incomingMessageIds) {
for (
final MapEntry(key: incomingStreamId, value: incomingTopics)
in newlyUnreadInStreams.entries
) {
for (
final MapEntry(key: incomingTopic, value: incomingMessageIds)
in incomingTopics.entries
) {
_addAllInStreamTopic(incomingMessageIds..sort(), incomingStreamId, incomingTopic);
});
});
newlyUnreadInDms.forEach((incomingDmNarrow, incomingMessageIds) {
}
}
for (
final MapEntry(key: incomingDmNarrow, value: incomingMessageIds)
in newlyUnreadInDms.entries
) {
_addAllInDm(incomingMessageIds..sort(), incomingDmNarrow);
});
}
}
}
notifyListeners();
Expand Down Expand Up @@ -328,21 +337,21 @@ class Unreads extends ChangeNotifier {
// TODO use efficient model lookups
void _slowRemoveAllInStreams(Set<int> idsToRemove) {
final newlyEmptyStreams = [];
streams.forEach((streamId, topics) {
for (final MapEntry(key: streamId, value: topics) in streams.entries) {
final newlyEmptyTopics = [];
topics.forEach((topic, messageIds) {
for (final MapEntry(key: topic, value: messageIds) in topics.entries) {
messageIds.removeWhere((id) => idsToRemove.contains(id));
if (messageIds.isEmpty) {
newlyEmptyTopics.add(topic);
}
});
}
for (final topic in newlyEmptyTopics) {
topics.remove(topic);
}
if (topics.isEmpty) {
newlyEmptyStreams.add(streamId);
}
});
}
for (final streamId in newlyEmptyStreams) {
streams.remove(streamId);
}
Expand Down Expand Up @@ -387,12 +396,12 @@ class Unreads extends ChangeNotifier {
// TODO use efficient model lookups
void _slowRemoveAllInDms(Set<int> idsToRemove) {
final newlyEmptyDms = [];
dms.forEach((dmNarrow, messageIds) {
for (final MapEntry(key: dmNarrow, value: messageIds) in dms.entries) {
messageIds.removeWhere((id) => idsToRemove.contains(id));
if (messageIds.isEmpty) {
newlyEmptyDms.add(dmNarrow);
}
});
}
for (final dmNarrow in newlyEmptyDms) {
dms.remove(dmNarrow);
}
Expand Down

0 comments on commit cc76f2a

Please sign in to comment.