Address full-codebase review: logging boundary, clone elimination, robustness
Library no longer prints to stderr: - Replace every eprintln! with log::warn! (lib: user_data, http; bin: parsing, output, commands); initialize env_logger in main with a warn-level default so output is unchanged unless RUST_LOG is set - Warn on missing/non-numeric "t" time fields instead of silently coercing them to 0.0; suppress the per-car NotFound warning (most cars legitimately have no timing file), keep warnings for real I/O errors Ownership and clone cleanup: - CarCounts no longer duplicates the car database: drop the car_list field and build car_map by consuming car_list (each Car stored once); iterate car_map.values() where the list was previously used - run_analysis takes MyCarsData by value: owned moves into the parse Arc, owned_configs/copy_count move into CarCounts; only the three commands that need owned/wishlist afterwards retain their own copy - is_final_build checks the digits directly (no per-call String allocation) and treats malformed configs as non-final so they surface in improve.csv; owned_final_configs becomes a borrowed rid-to-configs map so the target-exclusion check allocates nothing - Normalize sum arithmetic to u8 in normalize_to_final_build Docs and structure: - Document the first-seen-wins tie-breaking in compute_ideal_wishlist and the non-final/non-best coverage gap in write_trash_csv - Extract compare_by_rq_config_brand_name shared by compare_records and the remove.csv sort; drop the unused _output_dir parameter from run_track; add a retry_delay helper de-duplicating the three HTTP backoff sites; rename the shadowed `entry` binding to config_data
This commit is contained in:
parent
39d1e1d71b
commit
58b8d23e68
17 changed files with 255 additions and 120 deletions
|
|
@ -89,9 +89,10 @@ pub fn parse_car_times(
|
|||
// Read file content
|
||||
let file_content = match fs::read_to_string(&local_file_path) {
|
||||
Ok(content) => content,
|
||||
// File not found is expected for most cars (no timing data recorded) — silent skip.
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()),
|
||||
Err(e) => {
|
||||
// File not found is expected for some cars — treat as non-fatal
|
||||
eprintln!("Warning: could not read {local_file_path}: {e}");
|
||||
log::warn!("could not read {local_file_path}: {e}");
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
|
|
@ -106,7 +107,7 @@ pub fn parse_car_times(
|
|||
|
||||
// Process each configuration variant (111, 233, 323, etc.)
|
||||
if let Some(configurations) = json_data["data"].as_object() {
|
||||
for (raw_config, entry) in configurations {
|
||||
for (raw_config, config_data) in configurations {
|
||||
// Strip version prefix (e.g., "v2c_" from "v2c_233", "v2b_" from "v2b_111")
|
||||
// at read time. Downstream code only ever sees bare digit strings.
|
||||
// Generic approach: find the last '_' and take what follows it,
|
||||
|
|
@ -121,13 +122,20 @@ pub fn parse_car_times(
|
|||
.map(|(_, suffix)| suffix.to_string())
|
||||
.unwrap_or_else(|| raw_config.clone());
|
||||
// Process timing data for each track
|
||||
if let Some(times) = entry["times"].as_object() {
|
||||
if let Some(times) = config_data["times"].as_object() {
|
||||
for (track_id, details) in times {
|
||||
// Parse track ID to separate base track name from surface condition.
|
||||
// Use rsplit_once to split on the LAST "_a" so track names that
|
||||
// contain "_a" (e.g. "airplaneHangars") are handled correctly.
|
||||
if let Some((cleaned_track_id, track_type)) = track_id.rsplit_once("_a") {
|
||||
let mut time = details["t"].as_f64().unwrap_or(0.0);
|
||||
let Some(mut time) = details["t"].as_f64() else {
|
||||
log::warn!(
|
||||
"skipping {}/{}: missing or non-numeric time field",
|
||||
car.rid,
|
||||
track_id
|
||||
);
|
||||
continue;
|
||||
};
|
||||
|
||||
let down_votes = details["down"].as_array();
|
||||
let no_down_votes = match down_votes {
|
||||
|
|
@ -302,12 +310,12 @@ pub async fn parse_all_times(
|
|||
match result {
|
||||
// Outer JoinError from tokio::spawn
|
||||
Err(e) => {
|
||||
eprintln!("Warning: parsing task failed: {e}");
|
||||
log::warn!("parsing task failed: {e}");
|
||||
parse_errors += 1;
|
||||
}
|
||||
// parse_car_times or semaphore returned an error
|
||||
Ok(Err(e)) => {
|
||||
eprintln!("Warning: {e}");
|
||||
log::warn!("{e}");
|
||||
parse_errors += 1;
|
||||
}
|
||||
// parse_car_times succeeded
|
||||
|
|
@ -315,7 +323,7 @@ pub async fn parse_all_times(
|
|||
}
|
||||
}
|
||||
if parse_errors > 0 {
|
||||
eprintln!("Warning: {parse_errors} car(s) had parse errors during analysis");
|
||||
log::warn!("{parse_errors} car(s) had parse errors during analysis");
|
||||
}
|
||||
|
||||
// If every car failed to parse, the data is unusable — propagate the error.
|
||||
|
|
@ -364,14 +372,14 @@ pub async fn fetch_times(cars_db_path: &str, times_dir: &str) -> Result<(), AppE
|
|||
match result {
|
||||
Ok(Ok(())) => {}
|
||||
Ok(Err(e)) => {
|
||||
eprintln!("{e}");
|
||||
log::warn!("{e}");
|
||||
if first_error.is_none() {
|
||||
first_error = Some(e);
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
let msg = AppError::Network(format!("Task join error: {e}"));
|
||||
eprintln!("{msg}");
|
||||
log::warn!("{msg}");
|
||||
if first_error.is_none() {
|
||||
first_error = Some(msg);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue