Skip to content

Commit

Permalink
Simplify return type of get_exercise_batch (#294)
Browse files Browse the repository at this point in the history
  • Loading branch information
martinmr committed Nov 24, 2023
1 parent 349e299 commit 8ea407d
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 27 deletions.
6 changes: 3 additions & 3 deletions src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ pub trait ExerciseSchedulerFFI {
fn get_exercise_batch(
&self,
filter: Option<ExerciseFilter>,
) -> Result<Vec<(Ustr, ExerciseManifest)>, ExerciseSchedulerError>;
) -> Result<Vec<ExerciseManifest>, ExerciseSchedulerError>;
fn score_exercise(
&self,
exercise_id: Ustr,
Expand All @@ -130,12 +130,12 @@ impl ExerciseSchedulerFFI for TraneFFI {
fn get_exercise_batch(
&self,
filter: Option<ExerciseFilter>,
) -> Result<Vec<(Ustr, ExerciseManifest)>, ExerciseSchedulerError> {
) -> Result<Vec<ExerciseManifest>, ExerciseSchedulerError> {
Ok(self
.trane
.get_exercise_batch(filter.map(Into::into))?
.into_iter()
.map(|(unit_id, manifest)| (unit_id, manifest.into()))
.map(Into::into)
.collect())
}
fn score_exercise(
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ impl ExerciseScheduler for Trane {
fn get_exercise_batch(
&self,
filter: Option<ExerciseFilter>,
) -> Result<Vec<(Ustr, ExerciseManifest)>, ExerciseSchedulerError> {
) -> Result<Vec<ExerciseManifest>, ExerciseSchedulerError> {
self.scheduler.get_exercise_batch(filter)
}

Expand Down
10 changes: 5 additions & 5 deletions src/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub trait ExerciseScheduler {
fn get_exercise_batch(
&self,
filter: Option<ExerciseFilter>,
) -> Result<Vec<(Ustr, ExerciseManifest)>, ExerciseSchedulerError>;
) -> Result<Vec<ExerciseManifest>, ExerciseSchedulerError>;

/// Records the score of the given exercise's trial. The scores are used by the scheduler to
/// decide when to stop traversing a path and how to sort and filter all the found candidates
Expand Down Expand Up @@ -838,7 +838,7 @@ impl ExerciseScheduler for DepthFirstScheduler {
fn get_exercise_batch(
&self,
filter: Option<ExerciseFilter>,
) -> Result<Vec<(Ustr, ExerciseManifest)>, ExerciseSchedulerError> {
) -> Result<Vec<ExerciseManifest>, ExerciseSchedulerError> {
// Retrieve an initial batch of candidates based on the type of the filter.
let initial_candidates = self
.get_initial_candidates(filter)
Expand All @@ -852,10 +852,10 @@ impl ExerciseScheduler for DepthFirstScheduler {
.map_err(ExerciseSchedulerError::GetExerciseBatch)?; // grcov-excl-line

// Increment the frequency of the exercises in the batch. These exercises will have a lower
// chance of being selected in the future so that exercises that have not been selected as
// chance of being selected in the future,so that exercises that have not been selected as
// often have a higher chance of being selected.
for (exercise_id, _) in &final_candidates {
self.data.increment_exercise_frequency(*exercise_id);
for exercise_manifest in &final_candidates {
self.data.increment_exercise_frequency(exercise_manifest.id);
}

Ok(final_candidates)
Expand Down
19 changes: 6 additions & 13 deletions src/scheduler/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use anyhow::Result;
use lazy_static::lazy_static;
use rand::{prelude::SliceRandom, thread_rng};
use ustr::{Ustr, UstrMap, UstrSet};
use ustr::{UstrMap, UstrSet};

use crate::{
data::{ExerciseManifest, MasteryWindow},
Expand Down Expand Up @@ -232,22 +232,18 @@ impl CandidateFilter {
}

/// Takes a list of candidates and returns a vector of tuples of exercises IDs and manifests.
fn candidates_to_exercises(
&self,
candidates: Vec<Candidate>,
) -> Result<Vec<(Ustr, ExerciseManifest)>> {
fn candidates_to_exercises(&self, candidates: Vec<Candidate>) -> Result<Vec<ExerciseManifest>> {
// Retrieve the manifests for each candidate.
let mut exercises = candidates
.into_iter()
.map(|c| -> Result<(Ustr, _)> {
.map(|c| -> Result<_> {
let manifest = self.data.get_exercise_manifest(c.exercise_id)?;
Ok((c.exercise_id, manifest))
Ok(manifest)
})
.collect::<Result<Vec<(Ustr, _)>>>()?; // grcov-excl-line
.collect::<Result<Vec<_>>>()?; // grcov-excl-line

// Shuffle the list one more time to add more randomness to the final batch.
exercises.shuffle(&mut thread_rng());

Ok(exercises)
}

Expand All @@ -270,10 +266,7 @@ impl CandidateFilter {

/// Takes a list of exercises and filters them so that the end result is a list of exercise
/// manifests which fit the mastery windows defined in the scheduler options.
pub fn filter_candidates(
&self,
candidates: &[Candidate],
) -> Result<Vec<(Ustr, ExerciseManifest)>> {
pub fn filter_candidates(&self, candidates: &[Candidate]) -> Result<Vec<ExerciseManifest>> {
// Find the batch size to use.
let options = &self.data.options;
let batch_size = Self::dynamic_batch_size(options.batch_size, candidates.len());
Expand Down
14 changes: 9 additions & 5 deletions src/testutil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ impl TraneSimulation {
completed_exercises += 1;

// If the batch is empty, try to get another batch. If this batch is also empty, break
// early to avoid falling into an infinite loop.
// early to avoid falling into an nfinite loop.
if batch.is_empty() {
batch = trane.get_exercise_batch(filter.clone())?;
if batch.is_empty() {
Expand All @@ -467,12 +467,16 @@ impl TraneSimulation {
}

// Retrieve an exercise, compute its score, add it to the history, and submit it.
let (exercise_id, _) = batch.pop().unwrap();
let score = (self.answer_closure)(&exercise_id);
let exercise_manifest = batch.pop().unwrap();
let score = (self.answer_closure)(&exercise_manifest.id);
if let Some(score) = score {
trane.score_exercise(exercise_id, score.clone(), Utc::now().timestamp())?;
trane.score_exercise(
exercise_manifest.id,
score.clone(),
Utc::now().timestamp(),
)?; // grcov-excl-line
self.answer_history
.entry(exercise_id)
.entry(exercise_manifest.id)
.or_default()
.push(score);
}
Expand Down

0 comments on commit 8ea407d

Please sign in to comment.