Skip to content

Commit f09d0ea

Browse files
committed
chore(ui): Introduce the notion of _rank_ in the recency sorter.
This patch adds the notion of _rank_ in the `recency` sorter to avoid confusion around `u64`: is it a timestamp or a recency stamp? It's purely semantics, but I hope it clarify the code.
1 parent 1cd283e commit f09d0ea

File tree

1 file changed

+26
-23
lines changed
  • crates/matrix-sdk-ui/src/room_list_service/sorters

1 file changed

+26
-23
lines changed

crates/matrix-sdk-ui/src/room_list_service/sorters/recency.rs

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@ use super::{Room, Sorter};
2020

2121
struct RecencySorter<F>
2222
where
23-
F: Fn(&Room, &Room) -> (Option<u64>, Option<u64>),
23+
F: Fn(&Room, &Room) -> (Option<Rank>, Option<Rank>),
2424
{
25-
recency_stamps: F,
25+
ranks: F,
2626
}
2727

2828
impl<F> RecencySorter<F>
2929
where
30-
F: Fn(&Room, &Room) -> (Option<u64>, Option<u64>),
30+
F: Fn(&Room, &Room) -> (Option<Rank>, Option<Rank>),
3131
{
3232
fn cmp(&self, left: &Room, right: &Room) -> Ordering {
3333
if left.room_id() == right.room_id() {
@@ -51,8 +51,8 @@ where
5151
return Ordering::Greater;
5252
}
5353

54-
match (self.recency_stamps)(left, right) {
55-
(Some(left_stamp), Some(right_stamp)) => left_stamp.cmp(&right_stamp).reverse(),
54+
match (self.ranks)(left, right) {
55+
(Some(left_rank), Some(right_rank)) => left_rank.cmp(&right_rank).reverse(),
5656

5757
(Some(_), None) => Ordering::Less,
5858

@@ -72,21 +72,25 @@ where
7272
/// [`RoomInfo::recency_stamp`]: matrix_sdk_base::RoomInfo::recency_stamp
7373
/// [`RoomInfo::new_latest_event`]: matrix_sdk_base::RoomInfo::new_latest_event
7474
pub fn new_sorter() -> impl Sorter {
75-
let sorter =
76-
RecencySorter { recency_stamps: move |left, right| extract_recency_stamp(left, right) };
75+
let sorter = RecencySorter { ranks: move |left, right| extract_rank(left, right) };
7776

7877
move |left, right| -> Ordering { sorter.cmp(left, right) }
7978
}
8079

81-
/// Extract the recency stamp from either the [`RoomInfo::new_latest_event`] or
80+
/// The term _rank_ is used here to avoid any confusion with a _timestamp_ (a
81+
/// `u64` from the latest event), or a _recency stamp_ (a `u64` from the recency
82+
/// stamp of the room). This type hides `u64` for the sake of semantics.
83+
type Rank = u64;
84+
85+
/// Extract the recency _rank_ from either the [`RoomInfo::new_latest_event`] or
8286
/// from [`RoomInfo::recency_stamp`].
8387
///
84-
/// We must be very careful to return data of the same nature: either a _recency
85-
/// stamp_ from the [`LatestEventValue`], or from the
88+
/// We must be very careful to return data of the same nature: either a
89+
/// _rank_ from the [`LatestEventValue`]'s timestamp, or from the
8690
/// [`RoomInfo::recency_stamp`], but we **must never** mix both. The
8791
/// `RoomInfo::recency_stamp` is not a timestamp, while `LatestEventValue` uses
8892
/// a timestamp.
89-
fn extract_recency_stamp(left: &Room, right: &Room) -> (Option<u64>, Option<u64>) {
93+
fn extract_rank(left: &Room, right: &Room) -> (Option<Rank>, Option<Rank>) {
9094
match (left.new_latest_event(), right.new_latest_event()) {
9195
// None of both rooms, or only one of both rooms, have a latest event value. Let's fallback
9296
// to the recency stamp from the `RoomInfo` for both room.
@@ -96,8 +100,7 @@ fn extract_recency_stamp(left: &Room, right: &Room) -> (Option<u64>, Option<u64>
96100
(left.recency_stamp().map(Into::into), right.recency_stamp().map(Into::into))
97101
}
98102

99-
// Both rooms have a non-`None` latest event. We can use their timestamps as a recency
100-
// stamp.
103+
// Both rooms have a non-`None` latest event. We can use their timestamps as a rank.
101104
(
102105
left @ LatestEventValue::Remote(_)
103106
| left @ LatestEventValue::LocalIsSending(_)
@@ -202,23 +205,23 @@ mod tests {
202205
set_latest_event_value(&mut room_a, none());
203206
set_latest_event_value(&mut room_b, none());
204207

205-
assert_eq!(extract_recency_stamp(&room_a, &room_b), (Some(1), Some(2)));
208+
assert_eq!(extract_rank(&room_a, &room_b), (Some(1), Some(2)));
206209
}
207210

208211
// `room_a` has `None`, `room_b` has something else.
209212
{
210213
set_latest_event_value(&mut room_a, none());
211214
set_latest_event_value(&mut room_b, remote(3));
212215

213-
assert_eq!(extract_recency_stamp(&room_a, &room_b), (Some(1), Some(2)));
216+
assert_eq!(extract_rank(&room_a, &room_b), (Some(1), Some(2)));
214217
}
215218

216219
// `room_b` has `None`, `room_a` has something else.
217220
{
218221
set_latest_event_value(&mut room_a, remote(3));
219222
set_latest_event_value(&mut room_b, none());
220223

221-
assert_eq!(extract_recency_stamp(&room_a, &room_b), (Some(1), Some(2)));
224+
assert_eq!(extract_rank(&room_a, &room_b), (Some(1), Some(2)));
222225
}
223226
}
224227

@@ -240,7 +243,7 @@ mod tests {
240243
set_latest_event_value(&mut room_a, latest_event_value_a.clone());
241244
set_latest_event_value(&mut room_b, latest_event_value_b);
242245

243-
assert_eq!(extract_recency_stamp(&room_a, &room_b), (Some(3), Some(4)));
246+
assert_eq!(extract_rank(&room_a, &room_b), (Some(3), Some(4)));
244247
}
245248
}
246249
}
@@ -254,23 +257,23 @@ mod tests {
254257

255258
// `room_a` has an older recency stamp than `room_b`.
256259
{
257-
let sorter = RecencySorter { recency_stamps: |_left, _right| (Some(1), Some(2)) };
260+
let sorter = RecencySorter { ranks: |_left, _right| (Some(1), Some(2)) };
258261

259262
// `room_a` is greater than `room_b`, i.e. it must come after `room_b`.
260263
assert_eq!(sorter.cmp(&room_a, &room_b), Ordering::Greater);
261264
}
262265

263266
// `room_b` has an older recency stamp than `room_a`.
264267
{
265-
let sorter = RecencySorter { recency_stamps: |_left, _right| (Some(2), Some(1)) };
268+
let sorter = RecencySorter { ranks: |_left, _right| (Some(2), Some(1)) };
266269

267270
// `room_a` is less than `room_b`, i.e. it must come before `room_b`.
268271
assert_eq!(sorter.cmp(&room_a, &room_b), Ordering::Less);
269272
}
270273

271274
// `room_a` has an equally old recency stamp than `room_b`.
272275
{
273-
let sorter = RecencySorter { recency_stamps: |_left, _right| (Some(1), Some(1)) };
276+
let sorter = RecencySorter { ranks: |_left, _right| (Some(1), Some(1)) };
274277

275278
assert_eq!(sorter.cmp(&room_a, &room_b), Ordering::Equal);
276279
}
@@ -284,14 +287,14 @@ mod tests {
284287

285288
// `room_a` has a recency stamp, `room_b` has no recency stamp.
286289
{
287-
let sorter = RecencySorter { recency_stamps: |_left, _right| (Some(1), None) };
290+
let sorter = RecencySorter { ranks: |_left, _right| (Some(1), None) };
288291

289292
assert_eq!(sorter.cmp(&room_a, &room_b), Ordering::Less);
290293
}
291294

292295
// `room_a` has no recency stamp, `room_b` has a recency stamp.
293296
{
294-
let sorter = RecencySorter { recency_stamps: |_left, _right| (None, Some(1)) };
297+
let sorter = RecencySorter { ranks: |_left, _right| (None, Some(1)) };
295298

296299
assert_eq!(sorter.cmp(&room_a, &room_b), Ordering::Greater);
297300
}
@@ -305,7 +308,7 @@ mod tests {
305308

306309
// `room_a` and `room_b` has no recency stamp.
307310
{
308-
let sorter = RecencySorter { recency_stamps: |_left, _right| (None, None) };
311+
let sorter = RecencySorter { ranks: |_left, _right| (None, None) };
309312

310313
assert_eq!(sorter.cmp(&room_a, &room_b), Ordering::Equal);
311314
}

0 commit comments

Comments
 (0)