Implement overhead allocation for different account types, add it to binaries

FastCoster/CosterRs/coster-bridge.h

@@ -8,6 +8,6 @@
 #ifndef coster_bridge_h
 #define coster_bridge_h
 
-#import "libcoster_rs.h"
+#import "coster.h"
 
 #endif /* coster_bridge_h */

FastCoster/CosterRs/libcoster_rs.h

@@ -1,8 +0,0 @@
-#include <stdarg.h>
-#include <stdbool.h>
-#include <stdint.h>
-#include <stdlib.h>
-
-char *move_money_from_text(const char *rules, const char *lines, bool use_numeric_accounts);
-
-void move_money_from_text_free(char *s);

FastCoster/FastCoster.xcodeproj/project.pbxproj

@@ -7,6 +7,9 @@
 	objects = {
 
 /* Begin PBXBuildFile section */
+		5A1986F72996436500FA0471 /* OverheadAllocation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5A1986F62996436500FA0471 /* OverheadAllocation.swift */; };
+		5A1986F92996436D00FA0471 /* MoveMoney.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5A1986F82996436D00FA0471 /* MoveMoney.swift */; };
+		5A1986FB2996502C00FA0471 /* FileButtonSelector.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5A1986FA2996502C00FA0471 /* FileButtonSelector.swift */; };
 		5A450751298CE6D500E3D402 /* CsvDocument.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5A450750298CE6D500E3D402 /* CsvDocument.swift */; };
 		5A45075B298D01EF00E3D402 /* libcoster_rs.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 5A45075A298D01EF00E3D402 /* libcoster_rs.a */; };
 		5ADD9F2D298A713300F998F5 /* FastCosterApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5ADD9F2C298A713300F998F5 /* FastCosterApp.swift */; };
@@ -36,12 +39,15 @@
 /* End PBXContainerItemProxy section */
 
 /* Begin PBXFileReference section */
+		5A1986F62996436500FA0471 /* OverheadAllocation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OverheadAllocation.swift; sourceTree = "<group>"; };
+		5A1986F82996436D00FA0471 /* MoveMoney.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MoveMoney.swift; sourceTree = "<group>"; };
+		5A1986FA2996502C00FA0471 /* FileButtonSelector.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileButtonSelector.swift; sourceTree = "<group>"; };
+		5A1986FD29965BED00FA0471 /* coster.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = coster.h; path = ../../coster.h; sourceTree = "<group>"; };
 		5A450750298CE6D500E3D402 /* CsvDocument.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CsvDocument.swift; sourceTree = "<group>"; };
 		5A450754298CFFAB00E3D402 /* coster-bridge.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "coster-bridge.h"; sourceTree = "<group>"; };
 		5A450755298CFFE400E3D402 /* create-lib.sh */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; path = "create-lib.sh"; sourceTree = "<group>"; };
 		5A450756298D00AE00E3D402 /* remove-lib.sh */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; path = "remove-lib.sh"; sourceTree = "<group>"; };
 		5A45075A298D01EF00E3D402 /* libcoster_rs.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libcoster_rs.a; path = "../costerrs/target/aarch64-apple-ios/release/libcoster_rs.a"; sourceTree = "<group>"; };
-		5ACE47E6298D087B00834311 /* libcoster_rs.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = libcoster_rs.h; sourceTree = "<group>"; };
 		5ADD9F29298A713300F998F5 /* FastCoster.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = FastCoster.app; sourceTree = BUILT_PRODUCTS_DIR; };
 		5ADD9F2C298A713300F998F5 /* FastCosterApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FastCosterApp.swift; sourceTree = "<group>"; };
 		5ADD9F2E298A713300F998F5 /* ContentView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContentView.swift; sourceTree = "<group>"; };
@@ -85,7 +91,7 @@
 			isa = PBXGroup;
 			children = (
 				5A450754298CFFAB00E3D402 /* coster-bridge.h */,
-				5ACE47E6298D087B00834311 /* libcoster_rs.h */,
+				5A1986FD29965BED00FA0471 /* coster.h */,
 				5A450755298CFFE400E3D402 /* create-lib.sh */,
 				5A450756298D00AE00E3D402 /* remove-lib.sh */,
 			);
@@ -131,6 +137,9 @@
 				5ADD9F32298A713400F998F5 /* FastCoster.entitlements */,
 				5ADD9F33298A713400F998F5 /* Preview Content */,
 				5A450750298CE6D500E3D402 /* CsvDocument.swift */,
+				5A1986F62996436500FA0471 /* OverheadAllocation.swift */,
+				5A1986F82996436D00FA0471 /* MoveMoney.swift */,
+				5A1986FA2996502C00FA0471 /* FileButtonSelector.swift */,
 			);
 			path = FastCoster;
 			sourceTree = "<group>";
@@ -310,7 +319,7 @@
 		5A450758298D014E00E3D402 /* Remove Rust Lib */ = {
 			isa = PBXShellScriptBuildPhase;
 			alwaysOutOfDate = 1;
-			buildActionMask = 2147483647;
+			buildActionMask = 8;
 			files = (
 			);
 			inputFileListPaths = (
@@ -322,7 +331,7 @@
 			);
 			outputPaths = (
 			);
-			runOnlyForDeploymentPostprocessing = 0;
+			runOnlyForDeploymentPostprocessing = 1;
 			shellPath = /bin/sh;
 			shellScript = "source ${PROJECT_DIR}/CosterRs/remove-lib.sh\n";
 			showEnvVarsInLog = 0;
@@ -334,9 +343,12 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				5A1986FB2996502C00FA0471 /* FileButtonSelector.swift in Sources */,
 				5ADD9F2F298A713300F998F5 /* ContentView.swift in Sources */,
+				5A1986F92996436D00FA0471 /* MoveMoney.swift in Sources */,
 				5ADD9F2D298A713300F998F5 /* FastCosterApp.swift in Sources */,
 				5A450751298CE6D500E3D402 /* CsvDocument.swift in Sources */,
+				5A1986F72996436500FA0471 /* OverheadAllocation.swift in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

FastCoster/FastCoster/ContentView.swift

@@ -7,90 +7,37 @@
 
 import SwiftUI
 
+enum ProcessType: String, Hashable {
+    case MoveMoney = "Move Money"
+    case OverheadAllocation = "Overhead Allocation"
+    
+    static let values = [MoveMoney, OverheadAllocation]
+}
+
 struct ContentView: View {
-    @State private var showRulesPicker = false
+    
-    @State private var showLinesPicker = false
+    @State var selectedProcess: ProcessType?
-    @State private var showExportPicker = false
+
-    @State private var document: CsvDocument?
-    @State private var lines: String?
-    @State private var rules: String?
     var body: some View {
-        VStack {
+        NavigationSplitView {
-            Text ("Fast Coster").font(.largeTitle)
+            List(ProcessType.values, id: \.self, selection: $selectedProcess) { process in
-            Spacer()
+                NavigationLink(process.rawValue, value: process)
-            Button {
-                showRulesPicker.toggle()
-            } label: {
-                Text("Select Rules File")
-            }.fileImporter(isPresented: $showRulesPicker, allowedContentTypes: [.commaSeparatedText]) { result in
-                
-                switch result {
-                case .success(let fileUrl):
-                    rules = readIntoString(selectedFile: fileUrl)
-                case .failure(let error):
-                    print(error)
             }
-            }.padding()
+        } detail: {
-            Button {
+            if let process = selectedProcess {
-                showLinesPicker.toggle()
+                switch process {
-            } label: {
+                case .MoveMoney:
-                Text("Select Lines File")
+                    MoveMoney()
-            }.fileImporter(isPresented: $showLinesPicker, allowedContentTypes: [.commaSeparatedText]) { result in
-                switch result {
-                case .success(let fileUrl):
-                    lines = readIntoString(selectedFile: fileUrl)
-                case .failure(let error):
-                    print(error)
-                }
-            }.padding()
-            Button {
-                move_money()
-            } label: {
-                Text("Move money")
-            }.padding()
-            Spacer()
-        }
-        .padding()
-        .fileExporter(isPresented: $showExportPicker, document: document, contentType: .commaSeparatedText) { result in
-            
-            if case .success = result {
                 
+                case .OverheadAllocation:
+                    OverheadAllocation()
+                }
             } else {
-                
+                MoveMoney()
-            }
-            
-        }
-    }
-    
-    func move_money() {
-        DispatchQueue.global(qos: .userInitiated).async {
-            // Run move money
-            let result = move_money_from_text(rules, lines, true)
-            
-            DispatchQueue.main.async {
-                document = CsvDocument(data: String(cString: result!))
-                move_money_from_text_free(result)
-                showExportPicker = true;
             }
         }
     }
     
-    func readIntoString(selectedFile: URL) -> String {
-        // https://stackoverflow.com/questions/64118577/getting-the-file-xxx-couldnt-be-opened-because-you-dont-have-permission-to-v
-        do {
-            if selectedFile.startAccessingSecurityScopedResource() {
-                let fileContent = try String(contentsOf: selectedFile)
-                defer { selectedFile.stopAccessingSecurityScopedResource() }
-                return fileContent
-            }
-        }catch let error {
-            // TODO: Do something about this
-            print(error)
-        }
-        return ""
-    }
-    
-    
 }
 
 struct ContentView_Previews: PreviewProvider {

FastCoster/FastCoster/FileButtonSelector.swift

@@ -0,0 +1,53 @@
+//
+//  FileButtonSelector.swift
+//  FastCoster
+//
+//  Created by Michael Pivato on 10/2/2023.
+//
+
+import SwiftUI
+
+struct FileButtonSelector: View {
+    @State private var showPicker = false
+    var label: String
+    var onSelected: (String) -> ()
+    
+    var body: some View {
+        Button {
+            showPicker.toggle()
+        } label: {
+            Text(label)
+        }.fileImporter(isPresented: $showPicker, allowedContentTypes: [.commaSeparatedText]) { result in
+            
+            switch result {
+            case .success(let fileUrl):
+                onSelected(readIntoString(selectedFile: fileUrl))
+            case .failure(let error):
+                print(error)
+            }
+        }.padding()
+    }
+    
+    func readIntoString(selectedFile: URL) -> String {
+        // https://stackoverflow.com/questions/64118577/getting-the-file-xxx-couldnt-be-opened-because-you-dont-have-permission-to-v
+        do {
+            if selectedFile.startAccessingSecurityScopedResource() {
+                let fileContent = try String(contentsOf: selectedFile)
+                defer { selectedFile.stopAccessingSecurityScopedResource() }
+                return fileContent
+            }
+        }catch let error {
+            // TODO: Do something about this
+            print(error)
+        }
+        return ""
+    }
+}
+
+struct FileButtonSelector_Previews: PreviewProvider {
+    static var previews: some View {
+        FileButtonSelector(label: "Select File"){ response in
+            
+        }
+    }
+}

FastCoster/FastCoster/MoveMoney.swift

@@ -0,0 +1,61 @@
+//
+//  MoveMoney.swift
+//  FastCoster
+//
+//  Created by Michael Pivato on 10/2/2023.
+//
+
+import SwiftUI
+
+struct MoveMoney: View {
+    
+    @State private var showExportPicker = false
+    @State private var document: CsvDocument?
+    @State private var lines: String?
+    @State private var rules: String?
+    var body: some View {
+        VStack {
+            FileButtonSelector(label: "Select Rules File") { result in
+                rules = result
+            }
+            FileButtonSelector(label: "Select Lines File") { result in
+                lines = result
+            }
+            Button {
+                move_money()
+            } label: {
+                Text("Move money")
+            }.padding()
+                .fileExporter(isPresented: $showExportPicker, document: document, contentType: .commaSeparatedText) { result in
+                    
+                    if case .success = result {
+                        
+                    }else {
+                        
+                    }
+                    
+                }
+        }
+        .padding()
+        
+    }
+    
+    func move_money() {
+        DispatchQueue.global(qos: .userInitiated).async {
+            // Run move money
+            let result = move_money_from_text(rules, lines, true)
+            
+            DispatchQueue.main.async {
+                document = CsvDocument(data: String(cString: result!))
+                move_money_from_text_free(result)
+                showExportPicker = true;
+            }
+        }
+    }
+}
+
+struct MoveMoney_Previews: PreviewProvider {
+    static var previews: some View {
+        MoveMoney()
+    }
+}

FastCoster/FastCoster/OverheadAllocation.swift

@@ -0,0 +1,73 @@
+//
+//  OverheadAllocation.swift
+//  FastCoster
+//
+//  Created by Michael Pivato on 10/2/2023.
+//
+
+import SwiftUI
+
+struct OverheadAllocation: View {
+    @State private var lines: String?
+    @State private var accounts: String?
+    @State private var areas: String?
+    @State private var allocationStatistics: String?
+    @State private var costCentres: String?
+    @State private var showExportPicker = false
+    @State private var document: CsvDocument?
+    @State private var accountType = "E"
+    
+    var body: some View {
+        VStack {
+            FileButtonSelector(label: "Select Lines File") {result in
+                lines = result
+            }
+            FileButtonSelector(label: "Select Accounts File") {result in
+                accounts = result
+            }
+            FileButtonSelector(label: "Select Areas File") {result in
+                areas = result
+            }
+            FileButtonSelector(label: "Select Allocation Statistics File") {result in
+                allocationStatistics = result
+            }
+            FileButtonSelector(label: "Select Cost Centres File") {result in
+                costCentres = result
+            }
+            TextField("Account Type", text: $accountType)
+            Button {
+                allocate_overheads()
+            } label: {
+                Text("Allocate Overheads")
+            }.padding()
+                .fileExporter(isPresented: $showExportPicker, document: document, contentType: .commaSeparatedText) { result in
+                    
+                    if case .success = result {
+                        
+                    }else {
+                        
+                    }
+                    
+                }
+        }
+    }
+    
+    func allocate_overheads() {
+        DispatchQueue.global(qos: .userInitiated).async {
+            // Run move money
+            let result = allocate_overheads_from_text(lines, accounts, allocationStatistics, areas, costCentres, accountType);
+            
+            DispatchQueue.main.async {
+                document = CsvDocument(data: String(cString: result!))
+                allocate_overheads_from_text_free(result)
+                showExportPicker = true;
+            }
+        }
+    }
+}
+
+struct OverheadAllocation_Previews: PreviewProvider {
+    static var previews: some View {
+        OverheadAllocation()
+    }
+}

src/lib.rs

@@ -54,3 +54,65 @@ pub extern "C" fn move_money_from_text_free(s: *mut c_char) {
         CString::from_raw(s)
     };
 }
+
+#[no_mangle]
+pub extern "C" fn allocate_overheads_from_text(
+    lines: *const c_char,
+    accounts: *const c_char,
+    allocation_statistics: *const c_char,
+    areas: *const c_char,
+    cost_centres: *const c_char,
+    account_type: *const c_char,
+) -> *mut c_char {
+    let lines = unsafe {
+        assert!(!lines.is_null());
+        CStr::from_ptr(lines)
+    };
+    let accounts = unsafe {
+        assert!(!accounts.is_null());
+        CStr::from_ptr(accounts)
+    };
+    let allocation_statistics = unsafe {
+        assert!(!allocation_statistics.is_null());
+        CStr::from_ptr(allocation_statistics)
+    };
+    let areas = unsafe {
+        assert!(!areas.is_null());
+        CStr::from_ptr(areas)
+    };
+    let cost_centres = unsafe {
+        assert!(!cost_centres.is_null());
+        CStr::from_ptr(cost_centres)
+    };
+    let account_type = unsafe {
+        assert!(!account_type.is_null());
+        CStr::from_ptr(account_type)
+    };
+    let mut output_writer = csv::Writer::from_writer(vec![]);
+    reciprocal_allocation(
+        csv::Reader::from_reader(lines.to_bytes()),
+        csv::Reader::from_reader(accounts.to_bytes()),
+        csv::Reader::from_reader(allocation_statistics.to_bytes()),
+        csv::Reader::from_reader(areas.to_bytes()),
+        csv::Reader::from_reader(cost_centres.to_bytes()),
+        &mut output_writer,
+        true,
+        false,
+        true,
+        account_type.to_str().unwrap().to_owned(),
+    );
+    let inner = output_writer.into_inner().unwrap();
+    CString::new(String::from_utf8(inner).unwrap())
+        .unwrap()
+        .into_raw()
+}
+
+#[no_mangle]
+pub extern "C" fn allocate_overheads_from_text_free(s: *mut c_char) {
+    unsafe {
+        if s.is_null() {
+            return;
+        }
+        CString::from_raw(s)
+    };
+}

src/main.rs

@@ -41,10 +41,19 @@ enum Commands {
     /// Allocates servicing department amounts to operating departments
     allocate_overheads {
         #[clap(short, long, parse(from_os_str), value_name = "FILE")]
-        rules: PathBuf,
+        lines: PathBuf,
 
         #[clap(short, long, parse(from_os_str), value_name = "FILE")]
-        lines: PathBuf,
+        accounts: PathBuf,
+
+        #[clap(short, long, parse(from_os_str), value_name = "FILE")]
+        allocation_statistics: PathBuf,
+
+        #[clap(short, long, parse(from_os_str), value_name = "FILE")]
+        areas: PathBuf,
+
+        #[clap(short, long, parse(from_os_str), value_name = "FILE")]
+        cost_centres: PathBuf,
 
         #[clap(short, long, parse(from_os_str), value_name = "FILE")]
         output: Option<PathBuf>,
@@ -63,10 +72,20 @@ fn main() -> anyhow::Result<()> {
         } => move_money(rules, lines, output, use_numeric_accounts),
         Commands::smush_rules { rules, output } => smush_rules(rules, output),
         Commands::allocate_overheads {
-            rules,
             lines,
+            accounts,
+            allocation_statistics,
+            areas,
+            cost_centres,
             output,
-        } => allocate_overheads(rules, lines, output),
+        } => allocate_overheads(
+            lines,
+            accounts,
+            allocation_statistics,
+            areas,
+            cost_centres,
+            output,
+        ),
     }
 }
 
@@ -89,23 +108,25 @@ fn smush_rules(rules_path: PathBuf, output: Option<PathBuf>) -> anyhow::Result<(
 }
 
 fn allocate_overheads(
-    rules_path: PathBuf,
     lines: PathBuf,
+    accounts: PathBuf,
+    allocation_statistics: PathBuf,
+    areas: PathBuf,
+    cost_centres: PathBuf,
     output: Option<PathBuf>,
 ) -> anyhow::Result<()> {
-    let mut rdr = csv::Reader::from_path(rules_path)?;
+    coster_rs::reciprocal_allocation(
-
+        csv::Reader::from_path(lines)?,
-    for result in rdr.deserialize() {
+        csv::Reader::from_path(accounts)?,
-        let record: CsvOverheadAllocationRule = result?;
+        csv::Reader::from_path(allocation_statistics)?,
-    }
+        csv::Reader::from_path(areas)?,
-
+        csv::Reader::from_path(cost_centres)?,
-    let mut account_reader = csv::Reader::from_path(lines)?;
+        &mut csv::Writer::from_path(output.unwrap_or(PathBuf::from("alloc_output.csv")))?,
-
+        true,
-    for result in account_reader.deserialize() {
+        false,
-        let record: CsvCost = result?;
+        true,
-    }
+        "E".to_owned(),
-
+    )
-    Ok(())
 }
 
 #[derive(Debug, Deserialize)]

src/overhead_allocation.rs

@@ -104,6 +104,9 @@ pub fn reciprocal_allocation<Lines, Account, AllocationStatistic, Area, CostCent
     cost_centres: csv::Reader<CostCentre>,
     output: &mut csv::Writer<Output>,
     use_numeric_accounts: bool,
+    exclude_negative_allocation_statistics: bool,
+    any_limit_criteria: bool,
+    account_type: String,
 ) -> anyhow::Result<()>
 where
     Lines: Read,
@@ -120,10 +123,12 @@ where
 
     let mut accounts = accounts;
 
-    // TODO: Accounts need to come from actual account fiile
     let all_accounts_sorted: Vec<String> = if use_numeric_accounts {
         accounts
             .deserialize::<CsvAccount>()
+            .filter(|account| {
+                account.is_ok() && account.as_ref().unwrap().account_type == account_type
+            })
             .map(|line| line.unwrap().code.clone().parse::<i32>().unwrap())
             .unique()
             .sorted()
@@ -132,6 +137,9 @@ where
     } else {
         accounts
             .deserialize::<CsvAccount>()
+            .filter(|account| {
+                account.is_ok() && account.as_ref().unwrap().account_type == account_type
+            })
             .map(|line| line.unwrap().code.clone())
             .unique()
             .sorted()
@@ -140,63 +148,69 @@ where
 
     let mut allocation_statistics_reader = allocation_statistics;
     let allocation_statistics = allocation_statistics_reader
-        .deserialize()
+        .deserialize::<CsvAllocationStatistic>()
+        .filter(|allocation_statistic| {
+            allocation_statistic.as_ref().unwrap().account_type == account_type
+        })
         .collect::<Result<Vec<CsvAllocationStatistic>, csv::Error>>()?;
 
-    // For each allocation statistic, sum the cost centres across accounts in the allocaiton statistic range
+    let split_allocation_ranges: Vec<(String, Vec<AllocationStatisticAccountRange>)> =
-    // value is (cc, allocation_statistic, total)
+        allocation_statistics
-    // TODO: This is super slow
-    let flat_department_costs: HashMap<(String, String), f64> = allocation_statistics
             .iter()
             .map(|allocation_statistic| {
                 (
-                allocation_statistic,
+                    allocation_statistic.name.clone(),
                     split_allocation_statistic_range(allocation_statistic, &all_accounts_sorted),
                 )
             })
-        .flat_map(|allocation_statistic| {
+            .collect();
-            let mut total_department_costs: HashMap<String, f64> = HashMap::new();
+
-            lines
+    // For each allocation statistic, sum the cost centres across accounts in the allocaiton statistic range
-                .iter()
+    // value is (cc, allocation_statistic, total)
-                .filter(|line| {
+    let mut totals: HashMap<(String, String), f64> = HashMap::new();
+    for line in lines.iter() {
+        // TODO: Another optimisation potential here, puttinig the accounts into a map, although less important since there's usually <1k accounts
         let line_index = all_accounts_sorted
             .iter()
-                        .position(|account| account == &line.account)
+            .position(|account| account == &line.account);
-                        .unwrap();
+        // Skip account as it doesn't exist (likely due to wrong account type)
-                    allocation_statistic
+        if line_index.is_none() {
-                        .1
+            continue;
+        }
+        let line_index = line_index.unwrap();
+        // Find the allocation statistics this line is in
+        for (allocation_statistic, range) in split_allocation_ranges.iter() {
+            if range
                 .iter()
                 .find(|range| line_index >= range.start && line_index <= range.end)
                 .is_some()
-                })
+            {
-                .for_each(|line| {
+                *totals
-                    *total_department_costs
+                    .entry((line.department.clone(), allocation_statistic.clone()))
-                        .entry(line.department.clone())
                     .or_insert(0.) += line.value;
-                });
+            }
-            total_department_costs
+        }
-                .iter()
+    }
-                .map(|entry| {
-                    (
-                        (entry.0.clone(), allocation_statistic.0.name.clone()),
-                        *entry.1,
-                    )
-                })
-                .collect::<Vec<((String, String), f64)>>()
-        })
-        .collect();
-    // TODO: If ignore negative is used, then set values < 0 to 0
 
-    let mut rollups: HashMap<String, HashMap<String, Vec<String>>> = HashMap::new();
+    // If ignore negative is used, then set values < 0 to 0
+    if exclude_negative_allocation_statistics {
+        for ((_, _), total) in totals.iter_mut() {
+            if *total < 0. {
+                *total = 0.;
+            }
+        }
+    }
+
+    // Group ccs by area
     let mut area_ccs: HashMap<String, Vec<String>> = HashMap::new();
     let mut cost_centres = cost_centres;
     let headers = cost_centres.headers()?;
-    headers
+    // Group ccs by rollup, and group rollups into their slot
+    let mut rollups: HashMap<String, HashMap<String, Vec<String>>> = headers
         .iter()
         .filter(|name| name.to_lowercase().starts_with("rollupslot:"))
-        .for_each(|rollupslot| {
+        .map(|rollupslot| (rollupslot.to_owned(), HashMap::new()))
-            rollups.insert(rollupslot.to_owned(), HashMap::new());
+        .collect();
-        });
     for cost_centre in cost_centres.deserialize() {
         let cost_centre: HashMap<String, String> = cost_centre?;
         let name = cost_centre.get("Code").unwrap();
@@ -224,7 +238,11 @@ where
         .map(|header| header["limitto:".len()..].to_owned())
         .collect();
     let mut overhead_other_total: Vec<(String, String, f64)> = Vec::new();
+
+    // Save overhead ccs, so we later know whether a to cc is overhead or operating
     let mut overhead_ccs: Vec<String> = Vec::new();
+    // overhead department -> total (summed limit to costs)
+    let mut overhead_cc_totals: HashMap<String, f64> = HashMap::new();
     // For each overhead area, get the cost centres in the area, 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
@@ -240,16 +258,25 @@ where
         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 {
             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 assuming any limit criteria
+            // TODO: This depends on the area limit criteria. For now just doing any limit criteria
             let mut limited_ccs: Vec<String> = Vec::new();
             for limit_to in limit_tos.iter() {
-                // TODO: It is technically possible to have more than one limit to for a slot, so consider eventually splitting this and doing a foreach
+                // TODO: It is technically possible to have more than one limit to (I think?) for a slot, so consider eventually splitting this and doing a foreach
                 let limit_value = area.get(&("LimitTo:".to_owned() + limit_to)).unwrap();
                 if limit_value.is_empty() {
                     continue;
@@ -277,19 +304,34 @@ where
             let mut totals: Vec<(String, String, f64)> = overhead_ccs
                 .iter()
                 .flat_map(|overhead_cc| {
-                    limited_ccs
+                    let limited = limited_ccs
                         .iter()
+                        .filter(|other_cc| {
+                            totals.contains_key(&(
+                                // TODO: This looks terrible
+                                other_cc.clone().clone(),
+                                allocation_statistic.clone(),
+                            ))
+                        })
                         .map(|other_cc| {
                             (
                                 overhead_cc.clone(),
                                 other_cc.clone(),
-                                flat_department_costs
+                                totals
                                     .get(&(other_cc.clone(), allocation_statistic.clone()))
                                     .map(|f| *f)
-                                    .unwrap_or(0.),
+                                    .unwrap(),
                             )
                         })
                         .filter(|(_, _, value)| *value != 0.)
+                        .filter(|(from_cc, to_cc, _)| from_cc != to_cc)
+                        .collect_vec();
+                    // Insert is safe, since an overhead cc can only be a part of one area
+                    overhead_cc_totals.insert(
+                        overhead_cc.clone(),
+                        limited.iter().map(|(_, _, value)| value).sum(),
+                    );
+                    limited
                 })
                 .collect();
             overhead_other_total.append(&mut totals);
@@ -307,12 +349,7 @@ where
             |(from_overhead_department, to_department, percent)| OverheadAllocationRule {
                 from_overhead_department: from_overhead_department.clone(),
                 to_department: to_department.clone(),
-                percent: percent
+                percent: percent / overhead_cc_totals.get(from_overhead_department).unwrap(),
-                    / overhead_other_total
-                        .iter()
-                        .filter(|cc| cc.1 == *from_overhead_department)
-                        .map(|cc| cc.2)
-                        .sum::<f64>(),
                 to_department_type: if overhead_ccs.contains(&to_department) {
                     DepartmentType::Overhead
                 } else {
@@ -324,6 +361,12 @@ where
 
     let mut initial_account_costs: HashMap<String, Vec<TotalDepartmentCost>> = HashMap::new();
     for line in lines {
+        // Only include accounts we've already filtered on (i.e. by account type)
+        if all_accounts_sorted
+            .iter()
+            .find(|account| **account == line.account)
+            .is_some()
+        {
             initial_account_costs
                 .entry(line.account)
                 .or_insert(Vec::new())
@@ -332,6 +375,7 @@ where
                     value: line.value,
                 });
         }
+    }
 
     let results = reciprocal_allocation_impl(
         allocation_rules,
@@ -390,6 +434,8 @@ fn split_allocation_statistic_range(
         .collect()
 }
 
+// Removes quotes and padding from accounts int he allocation statistic account range.
+// e.g. "'100' " becomes "100"
 fn remove_quote_and_padding(s: &str) -> String {
     s.trim()[1..s.trim().len() - 1].to_owned()
 }
@@ -646,7 +692,10 @@ mod tests {
             csv::Reader::from_path("area.csv").unwrap(),
             csv::Reader::from_path("costcentre.csv").unwrap(),
             &mut csv::Writer::from_path("output_alloc_stat.csv").unwrap(),
+            true,
             false,
+            true,
+            "E".to_owned(),
         );
         assert!(result.is_ok())
     }