From db15c303791699ca0dc238c2f05ecf95bba740b0 Mon Sep 17 00:00:00 2001 From: Joel Therrien Date: Thu, 6 May 2021 20:35:49 -0700 Subject: [PATCH] Implement clippy suggestions --- src/bin/generator.rs | 75 +++++++++--------- src/bin/solver.rs | 28 +++---- src/generator.rs | 30 +++----- src/grid.rs | 63 +++++++-------- src/pdf.rs | 2 +- src/solver.rs | 179 ++++++++++++++++++------------------------- 6 files changed, 164 insertions(+), 213 deletions(-) diff --git a/src/bin/generator.rs b/src/bin/generator.rs index 63f8932..1562867 100644 --- a/src/bin/generator.rs +++ b/src/bin/generator.rs @@ -1,4 +1,5 @@ use rand::prelude::*; +use std::cmp::Ordering as ComparableOrdering; use std::error::Error; use std::io::Write; use std::process::exit; @@ -152,29 +153,31 @@ fn main() { let solve_controller = difficulty.map_to_solve_controller(); - let (result, num_attempts) = if threads < 1 { - eprintln!("--threads must be at least 1"); - exit(1); - } else if threads == 1 { - let mut rng = SmallRng::from_entropy(); - get_puzzle_matching_conditions( - &mut rng, - &difficulty, - &solve_controller, - max_attempts, - max_hints, - &AtomicBool::new(false), - debug, - ) - } else { - run_multi_threaded( + let (result, num_attempts) = match threads.cmp(&1) { + ComparableOrdering::Less => { + eprintln!("--threads must be at least 1"); + exit(1); + } + ComparableOrdering::Equal => { + let mut rng = SmallRng::from_entropy(); + get_puzzle_matching_conditions( + &mut rng, + &difficulty, + &solve_controller, + max_attempts, + max_hints, + &AtomicBool::new(false), + debug, + ) + } + ComparableOrdering::Greater => run_multi_threaded( max_attempts, max_hints, threads, debug, solve_controller, difficulty, - ) + ), }; let (grid, solve_statistics, num_hints) = match result { @@ -209,18 +212,15 @@ fn main() { println!("\t{} GUESS actions", solve_statistics.guesses); } - match filename { - Some(filename) => { - // check if we save to a csv or a pdf - if filename.ends_with(".pdf") { - sudoku_solver::pdf::draw_grid(&grid, &filename, print_possibilities).unwrap(); - println!("Grid saved as pdf to {}", filename); - } else { - save_grid_csv(&grid, &filename).unwrap(); - println!("Grid saved as CSV to {}", filename); - } + if let Some(filename) = filename { + // check if we save to a csv or a pdf + if filename.ends_with(".pdf") { + sudoku_solver::pdf::draw_grid(&grid, &filename, print_possibilities).unwrap(); + println!("Grid saved as pdf to {}", filename); + } else { + save_grid_csv(&grid, &filename).unwrap(); + println!("Grid saved as CSV to {}", filename); } - None => {} } } @@ -293,16 +293,13 @@ fn run_multi_threaded( let (result, attempts) = signal; attempt_count += attempts; - match result { - Some((safe_grid, solve_statistics, num_hints)) => { - result_to_return = Some((safe_grid.0, solve_statistics, num_hints)); - should_stop.store(true, Ordering::Relaxed); - } - None => {} - }; + if let Some((safe_grid, solve_statistics, num_hints)) = result { + result_to_return = Some((safe_grid.0, solve_statistics, num_hints)); + should_stop.store(true, Ordering::Relaxed); + } } - return (result_to_return, attempt_count); + (result_to_return, attempt_count) } fn get_puzzle_matching_conditions( @@ -330,7 +327,7 @@ fn get_puzzle_matching_conditions( } } - return (None, num_attempts); + (None, num_attempts) } fn save_grid_csv(grid: &Grid, filename: &str) -> Result<(), Box> { @@ -350,9 +347,9 @@ fn save_grid_csv(grid: &Grid, filename: &str) -> Result<(), Box> { if y < 8 { text.push(','); } - file.write(text.as_bytes())?; + file.write_all(text.as_bytes())?; } - file.write(b"\n")?; + file.write_all(b"\n")?; } Ok(()) diff --git a/src/bin/solver.rs b/src/bin/solver.rs index a787201..9d2a329 100644 --- a/src/bin/solver.rs +++ b/src/bin/solver.rs @@ -59,8 +59,7 @@ fn read_grid(filename: &str) -> Result { }; let grid = Grid::new(); - let mut row = 0; - for result in reader.records() { + for (row, result) in reader.records().enumerate() { if row > 8 { return Err("Hit row limit".to_string()); } @@ -69,24 +68,19 @@ fn read_grid(filename: &str) -> Result { for column in 0..9 { let value = record.get(column); - match value { - Some(x) => { - let digit_result = u8::from_str(x); - match digit_result { - Ok(digit) => { - if digit > 0 { - grid.get(row, column).unwrap().set(digit); - } + if let Some(x) = value { + let digit_result = u8::from_str(x); + match digit_result { + Ok(digit) => { + if digit > 0 { + grid.get(row, column).unwrap().set(digit); } - Err(_error) => return Err("Invalid cell value".to_string()), - }; - } - None => {} + } + Err(_error) => return Err("Invalid cell value".to_string()), + }; } } - - row = row + 1; } - return Ok(grid); + Ok(grid) } diff --git a/src/generator.rs b/src/generator.rs index 17359f7..dcb8918 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -76,17 +76,11 @@ impl Cell { for (_index, other) in line.vec.iter().enumerate() { if other.x != cell.x || other.y != cell.y { let value = &*other.value.borrow(); - match value { - CellValue::Fixed(digit) => { - let location = possibilities.binary_search(digit); - match location { - Ok(location) => { - possibilities.remove(location); - } - Err(_) => {} - } + if let CellValue::Fixed(digit) = value { + let location = possibilities.binary_search(digit); + if let Ok(location) = location { + possibilities.remove(location); } - CellValue::Unknown(_) => {} } } } @@ -108,7 +102,7 @@ impl Cell { self, ); - return possibilities; + possibilities } } @@ -152,20 +146,20 @@ pub fn generate_grid( let mut statistics_option = None; for (_index, cell) in non_empty_cells.iter().enumerate() { - let mut grid_clone = grid.clone(); + let grid_clone = grid.clone(); let cell_clone = grid_clone.get(cell.x, cell.y).unwrap(); let cell_clone = &*cell_clone; cell_clone.delete_value(); let (status, statistics) = - evaluate_grid_with_solve_controller(&mut grid_clone, solve_controller); + evaluate_grid_with_solve_controller(&grid_clone, solve_controller); match status { SolveStatus::Complete(uniqueness) => { let uniqueness = uniqueness.unwrap(); match uniqueness { Uniqueness::Unique => { - num_hints = num_hints - 1; + num_hints -= 1; grid = grid_clone; } Uniqueness::NotUnique => continue, // We can't remove this cell; continue onto the next one (note that grid hasn't been modified because of solve_controller) @@ -181,7 +175,7 @@ pub fn generate_grid( statistics_option = Some(statistics); } - return (grid, num_hints, statistics_option.unwrap()); + (grid, num_hints, statistics_option.unwrap()) } // We generate a completed grid with no mind for difficulty; afterward generate_puzzle will take out as many fields as it can with regards to the difficulty @@ -245,13 +239,13 @@ fn generate_completed_grid(rng: &mut SmallRng) -> Grid { cell_possibilities.shuffle(rng); for (_index, digit) in cell_possibilities.iter().enumerate() { - let mut grid_clone = grid.clone(); + let grid_clone = grid.clone(); let cell = &*grid_clone.get(cell.x, cell.y).unwrap(); cell.set(*digit); let (status, _statistics) = - evaluate_grid_with_solve_controller(&mut grid_clone, &solve_controller); + evaluate_grid_with_solve_controller(&grid_clone, &solve_controller); match status { SolveStatus::Complete(uniqueness) => { let uniqueness = uniqueness.unwrap(); @@ -279,7 +273,7 @@ fn generate_completed_grid(rng: &mut SmallRng) -> Grid { crate::solver::solve_grid(&mut grid); - return grid; + grid } #[cfg(test)] diff --git a/src/grid.rs b/src/grid.rs index 44296d4..c3957f0 100644 --- a/src/grid.rs +++ b/src/grid.rs @@ -71,7 +71,7 @@ impl Cell { /// Get a copy of the `CellValue` pub fn get_value_copy(&self) -> CellValue { let value = &*self.value.borrow(); - return value.clone(); + value.clone() } /// Set the cell value with a provided `CellValue`; if `value` is Fixed then the related cell's @@ -80,7 +80,6 @@ impl Cell { match value { CellValue::Fixed(digit) => { self.set(digit); - return; } CellValue::Unknown(_) => { self.set_value_exact(value); @@ -164,12 +163,9 @@ impl Cell { CellValue::Unknown(possibilities) => { let mut new_possibilities = possibilities.clone(); - match new_possibilities.binary_search(&digit) { - Ok(index_remove) => { - new_possibilities.remove(index_remove); - } - _ => {} - }; + if let Ok(index_remove) = new_possibilities.binary_search(&digit) { + new_possibilities.remove(index_remove); + } Some(CellValue::Unknown(new_possibilities)) /* @@ -186,11 +182,8 @@ impl Cell { } }; - match new_value_option { - Some(new_value) => { - cell.set_value(new_value); - } - None => {} + if let Some(new_value) = new_value_option { + cell.set_value(new_value); } } } @@ -238,7 +231,7 @@ impl Section { let do_update = &self.do_update.borrow(); let do_update = &**do_update; - return do_update.clone(); + *do_update } } @@ -305,11 +298,11 @@ impl Grid { } } - return Grid { + Grid { rows, columns, sections, - }; + } } /// Returns the `Cell` (in an `Rc`) at the specified coordinates. @@ -330,10 +323,10 @@ impl Grid { None => return None, }; - return Some(Rc::clone(cell)); + Some(Rc::clone(cell)) } - fn process_unknown(x: &Vec, digit: u8, row: &mut String) { + fn process_unknown(x: &[u8], digit: u8, row: &mut String) { if x.contains(&digit) { row.push('*'); } else { @@ -353,14 +346,11 @@ impl Grid { let cell = &*self.get(x, y).unwrap(); let cell_value = &*cell.value.borrow(); - match cell_value { - CellValue::Unknown(possibilities) => { - if (possibilities.len() < smallest_size) && (possibilities.len() > 0) { - smallest_size = possibilities.len(); - smallest_cell = Some(cell_rc); - } + if let CellValue::Unknown(possibilities) = cell_value { + if (possibilities.len() < smallest_size) && (!possibilities.is_empty()) { + smallest_size = possibilities.len(); + smallest_cell = Some(cell_rc); } - _ => {} } } } @@ -368,12 +358,18 @@ impl Grid { } } +impl Default for Grid { + fn default() -> Self { + Self::new() + } +} + impl Clone for Grid { fn clone(&self) -> Self { let mut new = Grid::new(); new.clone_from(&self); - return new; + new } fn clone_from(&mut self, source: &Self) { @@ -446,20 +442,17 @@ impl std::fmt::Display for Grid { } } - write!(f, "{}", row1)?; - write!(f, "\n")?; - write!(f, "{}", row2)?; - write!(f, "\n")?; - write!(f, "{}", row3)?; - write!(f, "\n")?; + writeln!(f, "{}", row1)?; + writeln!(f, "{}", row2)?; + writeln!(f, "{}", row3)?; if (r % 3 == 2) && (r < 8) { - write!(f, "━━━┿━━━┿━━━╋━━━┿━━━┿━━━╋━━━┿━━━┿━━━\n")?; + writeln!(f, "━━━┿━━━┿━━━╋━━━┿━━━┿━━━╋━━━┿━━━┿━━━")?; } else if r < 8 { - write!(f, "┄┄┄┼┄┄┄┼┄┄┄╂┄┄┄┼┄┄┄┼┄┄┄╂┄┄┄┼┄┄┄┼┄┄┄\n")?; + writeln!(f, "┄┄┄┼┄┄┄┼┄┄┄╂┄┄┄┼┄┄┄┼┄┄┄╂┄┄┄┼┄┄┄┼┄┄┄")?; } } - return Result::Ok(()); + Result::Ok(()) } } diff --git a/src/pdf.rs b/src/pdf.rs index a411d8a..f194553 100644 --- a/src/pdf.rs +++ b/src/pdf.rs @@ -67,7 +67,7 @@ pub fn draw_grid( doc.save(&mut BufWriter::new(File::create(filename)?))?; - return Ok(()); + Ok(()) } fn draw_empty_grid(layer: &PdfLayerReference) { diff --git a/src/solver.rs b/src/solver.rs index 4594263..d308d70 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -28,24 +28,19 @@ enum SolveAction { impl SolveStatus { fn increment(self, additional_status: SolveStatus) -> SolveStatus { match self { - SolveStatus::Complete(uniqueness_option) => { - if uniqueness_option.is_none() { - return SolveStatus::Complete(None); - } else { - match uniqueness_option.unwrap() { - Uniqueness::NotUnique => SolveStatus::Complete(Some(Uniqueness::NotUnique)), - Uniqueness::Unique => match additional_status { - SolveStatus::Complete(_) => { - SolveStatus::Complete(Some(Uniqueness::NotUnique)) - } - SolveStatus::Unfinished => { - SolveStatus::Complete(Some(Uniqueness::Unique)) - } - SolveStatus::Invalid => SolveStatus::Complete(Some(Uniqueness::Unique)), - }, - } - } - } + SolveStatus::Complete(uniqueness_option) => match uniqueness_option { + None => SolveStatus::Complete(None), + Some(uniqueness_option) => match uniqueness_option { + Uniqueness::NotUnique => SolveStatus::Complete(Some(Uniqueness::NotUnique)), + Uniqueness::Unique => match additional_status { + SolveStatus::Complete(_) => { + SolveStatus::Complete(Some(Uniqueness::NotUnique)) + } + SolveStatus::Unfinished => SolveStatus::Complete(Some(Uniqueness::Unique)), + SolveStatus::Invalid => SolveStatus::Complete(Some(Uniqueness::Unique)), + }, + }, + }, SolveStatus::Unfinished => match additional_status { SolveStatus::Invalid => SolveStatus::Unfinished, _ => additional_status, @@ -137,15 +132,21 @@ impl SolveStatistics { fn increment(&mut self, action: &SolveAction) { match action { - SolveAction::Single => self.singles = self.singles + 1, - SolveAction::HiddenSingle => self.hidden_singles = self.hidden_singles + 1, - SolveAction::PossibilityGroup => self.possibility_groups = self.possibility_groups + 1, - SolveAction::UsefulConstraints => self.useful_constraints = self.useful_constraints + 1, - SolveAction::Guess => self.guesses = self.guesses + 1, + SolveAction::Single => self.singles += 1, + SolveAction::HiddenSingle => self.hidden_singles += 1, + SolveAction::PossibilityGroup => self.possibility_groups += 1, + SolveAction::UsefulConstraints => self.useful_constraints += 1, + SolveAction::Guess => self.guesses += 1, } } } +impl Default for SolveStatistics { + fn default() -> Self { + Self::new() + } +} + // Code for identify_and_process_possibility_groups (it uses it's own structs) mod process_possibility_groups { use crate::grid::{CellValue, Section}; @@ -233,7 +234,7 @@ mod process_possibility_groups { CellValue::Unknown(possibilities) => { let mut set = HashSet::new(); for (_index, digit) in possibilities.iter().enumerate() { - set.insert(digit.clone()); + set.insert(*digit); } set } @@ -282,7 +283,7 @@ mod process_possibility_groups { smallest_cell.in_group = true; // Step 2 - count = count + smallest_size; + count += smallest_size; let possibilities_to_remove = smallest_cell.possibilities.clone(); // Necessary because of mutable borrow rules @@ -313,11 +314,11 @@ mod process_possibility_groups { for (_index, cell) in faux_line.0.iter().enumerate() { if cell.in_group { cell.possibilities.iter().for_each(|digit| { - in_group_possibilities.insert(digit.clone()); + in_group_possibilities.insert(digit); }); } else { cell.possibilities.iter().for_each(|digit| { - out_group_possibilities.insert(digit.clone()); + out_group_possibilities.insert(digit); }); } } @@ -342,12 +343,9 @@ mod process_possibility_groups { }; for (_i, possibility) in possibilities_to_remove.iter().enumerate() { - match possibilities.binary_search(possibility) { - Ok(x) => { - possibilities.remove(x); - } - Err(_) => {} - }; + if let Ok(x) = possibilities.binary_search(possibility) { + possibilities.remove(x); + } } if possibilities.len() < starting_possibility_size { @@ -384,7 +382,7 @@ mod process_possibility_groups { bisect_possibility_groups(line, out_group_indices); } - return made_change; + made_change } } @@ -402,19 +400,16 @@ fn search_single_possibility(line: &Section) -> bool { let mut made_change = false; for (_index, cell) in line.vec.iter().enumerate() { - match cell.get_value_possibilities() { - Some(x) => { - if x.len() == 1 { - let new_value = CellValue::Fixed(x.first().unwrap().clone()); - cell.set_value(new_value); - made_change = true; - } + if let Some(x) = cell.get_value_possibilities() { + if x.len() == 1 { + let new_value = CellValue::Fixed(*x.first().unwrap()); + cell.set_value(new_value); + made_change = true; } - None => {} } } - return made_change; + made_change } // Count up how many times each possibility occurs in the Line. If it only occurs once, that's a hidden single that we can set @@ -423,7 +418,7 @@ fn search_hidden_single(line: &Section) -> bool { None, One(Rc), Many, - }; + } impl Count { fn increment(&self, cell: Rc) -> Count { @@ -464,16 +459,13 @@ fn search_hidden_single(line: &Section) -> bool { } for (digit, count) in counts.iter().enumerate() { - match count { - Count::One(cell) => { - cell.set((digit + 1) as u8); - made_change = true; - } - _ => {} + if let Count::One(cell) = count { + cell.set((digit + 1) as u8); + made_change = true; } } - return made_change; + made_change } mod search_useful_constraint { @@ -489,10 +481,7 @@ mod search_useful_constraint { impl PossibilityLines { fn is_invalid(&self) -> bool { - match &self { - PossibilityLines::Invalid => true, - _ => false, - } + matches!(&self, PossibilityLines::Invalid) } } @@ -563,45 +552,33 @@ mod search_useful_constraint { } // Check each line and see if we can determine anything - match rows { - PossibilityLines::Unique(index) => { - made_change = made_change - | remove_possibilities_line( - grid.rows.get(index).unwrap(), - possibility, - &line.section_type, - line.index, - ); - } - _ => {} + if let PossibilityLines::Unique(index) = rows { + made_change |= remove_possibilities_line( + grid.rows.get(index).unwrap(), + possibility, + &line.section_type, + line.index, + ); } - match columns { - PossibilityLines::Unique(index) => { - made_change = made_change - | remove_possibilities_line( - grid.columns.get(index).unwrap(), - possibility, - &line.section_type, - line.index, - ); - } - _ => {} + if let PossibilityLines::Unique(index) = columns { + made_change |= remove_possibilities_line( + grid.columns.get(index).unwrap(), + possibility, + &line.section_type, + line.index, + ); } - match sections { - PossibilityLines::Unique(index) => { - made_change = made_change - | remove_possibilities_line( - grid.sections.get(index).unwrap(), - possibility, - &line.section_type, - line.index, - ); - } - _ => {} + if let PossibilityLines::Unique(index) = sections { + made_change |= remove_possibilities_line( + grid.sections.get(index).unwrap(), + possibility, + &line.section_type, + line.index, + ); } } - return made_change; + made_change } // initial_line_type and initial_line_index are to identify the cells that should NOT have their possibilities removed @@ -645,8 +622,7 @@ mod search_useful_constraint { let new_value; if new_possibilities.len() == 1 { - new_value = - CellValue::Fixed(new_possibilities.first().unwrap().clone()); + new_value = CellValue::Fixed(*new_possibilities.first().unwrap()); } else { new_value = CellValue::Unknown(new_possibilities); } @@ -663,7 +639,7 @@ mod search_useful_constraint { made_change = true; } - return made_change; + made_change } // We detected a useful constraint @@ -777,7 +753,7 @@ pub fn solve_grid(grid: &mut Grid) -> (SolveStatus, SolveStatistics) { let solve_status = solve_grid_with_solve_controller(grid, &solve_controller, &mut solve_statistics); - return (solve_status, solve_statistics); + (solve_status, solve_statistics) } /// Solves (and modifies) the input `Grid` & `SolveStatistics`. Returns a `SolveStatus`. @@ -805,7 +781,7 @@ pub fn solve_grid_with_solve_controller( _ => status, }; - return status; + status } /// Similar to `solve_grid_with_solve_controller` except that we don't modify the input Grid; we @@ -821,7 +797,7 @@ pub fn evaluate_grid_with_solve_controller( let solve_status = solve_grid_with_solve_controller(&mut mut_grid, solve_controller, &mut solve_statistics); - return (solve_status, solve_statistics); + (solve_status, solve_statistics) } fn solve_grid_no_guess( @@ -867,12 +843,12 @@ fn solve_grid_no_guess( for y in 0..9 { let cell = grid.get(x, y).unwrap(); let cell = &*cell; - let value = &**(&cell.value.borrow()); + let value = &*cell.value.borrow(); match value { CellValue::Unknown(possibilities) => { appears_complete = false; - if possibilities.len() == 0 { + if possibilities.is_empty() { return SolveStatus::Invalid; } } @@ -917,11 +893,8 @@ fn solve_grid_guess( solve_grid_with_solve_controller(&mut grid_copy, solve_controller, solve_statistics); // Keep a copy of grid_copy in case we later mutate grid with it - match status { - SolveStatus::Complete(_) => { - grid_solution = Some(grid_copy); - } - _ => {} + if let SolveStatus::Complete(_) = status { + grid_solution = Some(grid_copy); } current_status = current_status.increment(status); @@ -963,7 +936,7 @@ fn solve_grid_guess( SolveStatus::Invalid => {} } - return current_status; + current_status } #[cfg(test)]