[tech] Calendar deduplication #476
Conversation
src/model.rs
Outdated
| @@ -777,6 +777,39 @@ impl Collections { | |||
| } | |||
| self.vehicle_journeys = CollectionWithId::new(vehicle_journeys).unwrap(); | |||
| } | |||
|
|
|||
| /// Many calendars are identicall and can be deduplicate | |||
There was a problem hiding this comment.
| /// Many calendars are identicall and can be deduplicate | |
| /// Many calendars are identical and can be deduplicate |
src/model.rs
Outdated
|
|
||
| let mut calendars_used: Vec<Calendar> = vec![]; | ||
| let mut vehicle_journeys = self.vehicle_journeys.take(); | ||
| vehicle_journeys.sort_unstable_by_key(|vj| vj.id.clone()); |
There was a problem hiding this comment.
I'm not sure I understand why you need to sort the vehicle_journeys by id? vehicle_journeys and calendars are both stored in CollectionWithId which are already ordered and the order is not gonna be altered (only some elements are gonna be removed, but relative positions are gonna be the same). Or am I missing something?
If I am missing something and the sorting needs to stay, please consider the following solution to avoid the .clone() operation.
| vehicle_journeys.sort_unstable_by_key(|vj| vj.id.clone()); | |
| vehicle_journeys.sort_unstable_by(|vj_a, vj_b| { | |
| vj_a.service_id | |
| .partial_cmp(&vj_b.service_id) | |
| .unwrap_or(std::cmp::Ordering::Equal) | |
| }); |
There was a problem hiding this comment.
why not vehicle_journeys.sort_unstable_by(|vj1, vj2| vj1.service_id.cmp(&vj2.service_id)); ?
There was a problem hiding this comment.
Yes, why not? That's obviously better than mine! No reason, just didn't look deep enough I guess.
src/model.rs
Outdated
| for c in calendars { | ||
| if c.dates == *dates { | ||
| return Some(c); | ||
| } | ||
| } | ||
| None |
There was a problem hiding this comment.
This part could be written with the following syntax too.
calendars.iter().filter(|c| c.dates == *dates).next()This is shorter and might allow to remove the find_duplicate_calendar function and put the code directly inline. If you prefer to keep the find_duplicate_calendar, then maybe the for-loop is more readable?
There was a problem hiding this comment.
calendars.iter().find(|c| c.dates == *dates)
There was a problem hiding this comment.
Ah yes, once again, you caught it. Maybe one day I'll think about it!
patochectp
force-pushed
the
calendar_deduplication
branch
from
3164c9c to
29cc444
Compare
datanel
force-pushed
the
calendar_deduplication
branch
from
29cc444 to
4ed96f3
Compare
No description provided.