diff --git a/src/overhead_allocation.rs b/src/overhead_allocation.rs index 7896c4a..dca0cea 100644 --- a/src/overhead_allocation.rs +++ b/src/overhead_allocation.rs @@ -243,7 +243,7 @@ where let mut overhead_ccs: Vec = Vec::new(); // overhead department -> total (summed limit to costs) let mut overhead_cc_totals: HashMap = HashMap::new(); - // For each overhead area, get the cost centres in the area, and get all cost centres + // For each overhead area, get the cost centres in the area (overhead cost centres), and get all cost centres // that fit the limit to criteria for the area (skip any cases of overhead cc = other cc). // Then get the totals for the other ccs, by looking in the flat_department_costs, where the // allocation statistic matches the allocation statistic for this area @@ -253,24 +253,15 @@ where let area_name = area.get("Name").unwrap(); let allocation_statistic = area.get("AllocationStatistic").unwrap(); let department_type: DepartmentType = DepartmentType::from(area.get("Type").unwrap()); - let current_area_ccs = area_ccs.get(area_name); - if current_area_ccs.is_none() { - continue; - } - // Also skip if allocation statistic isn't found - // let allocation_statistic_found = allocation_statistics - // .iter() - // .find(|stat| stat.name == *allocation_statistic); - // if allocation_statistic_found.is_none() { - // continue; - // } - - let mut current_area_ccs = current_area_ccs.unwrap().clone(); - - //TODO: This isn't bad, however if it's too slow then consider performance improvements, such as summing totals now - // rather than calculating in the next step. if department_type == DepartmentType::Overhead { + let current_area_ccs = area_ccs.get(area_name); + + if current_area_ccs.is_none() { + continue; + } + let mut current_area_ccs = current_area_ccs.unwrap().clone(); + overhead_ccs.append(&mut current_area_ccs); let overhead_ccs = area_ccs.get(area_name).unwrap(); // TODO: This depends on the area limit criteria. For now just doing any limit criteria @@ -377,6 +368,12 @@ where } } + // TODO: (Consider) We could actually cheat here and not use this matrix implementation at all (and thus be more + // memory efficient, but maybe slower) + // Since we know each operating department in an account will get the proportion of the total overhead amount relative + // according to its operating amount from the total amount of the overhead departments, we can just directly calculate + // these totals and do some simple multiplications (it does get trickier with multiple accounts, as the cost drivers + // are consistent across all accounts, but depend on the allocation statistic to determine which lines to pick from). let results = reciprocal_allocation_impl( allocation_rules, initial_account_costs @@ -390,11 +387,14 @@ where for cost in results { for department in cost.summed_department_costs { - output.serialize(CsvCost { - account: cost.account.clone(), - department: department.department, - value: department.value, - })?; + // Any consumers should assume missing cc/account value was 0 (we already ignore overhead, as they all 0 out) + if department.value != 0. { + output.serialize(CsvCost { + account: cost.account.clone(), + department: department.department, + value: department.value, + })?; + } } } Ok(()) @@ -404,7 +404,7 @@ fn split_allocation_statistic_range( allocation_statistic: &CsvAllocationStatistic, accounts_sorted: &Vec, ) -> Vec { - // TODO: This split needs to be more comprehensive so that we don't split between quotes + // TODO: This split needs to be more comprehensive so that we don't split between quotes, so use a regex let split = allocation_statistic.account_ranges.split(";"); split .map(|split| { @@ -533,7 +533,7 @@ fn do_solve_reciprocal( .get(&rule.to_department) .unwrap(); operating_overhead_mappings - [from_index * overhead_department_mappings.len() + to_index] = rule.percent; + [from_index * operating_department_mappings.len() + to_index] = rule.percent; } } let operating_overhead_mappings_mat: DMatrix = DMatrix::from_vec( @@ -598,6 +598,7 @@ fn do_solve_reciprocal( mod tests { use crate::reciprocal_allocation; use crate::AccountCost; + use crate::CsvCost; use crate::DepartmentType; use crate::OverheadAllocationRule; use crate::TotalDepartmentCost; @@ -683,6 +684,23 @@ mod tests { assert_eq!(expected_final_allocations, result); } + #[test] + fn test_basic_real() { + let result = reciprocal_allocation( + csv::Reader::from_path("test_line.csv").unwrap(), + csv::Reader::from_path("test_account.csv").unwrap(), + csv::Reader::from_path("test_alloc_stat.csv").unwrap(), + csv::Reader::from_path("test_area.csv").unwrap(), + csv::Reader::from_path("test_costcentre.csv").unwrap(), + &mut csv::Writer::from_path("test_output_alloc_stat.csv").unwrap(), + true, + false, + true, + "E".to_owned(), + ); + assert!(result.is_ok()); + } + #[test] fn test_real() { let result = reciprocal_allocation(