From 1de467ec227b823ba919702ca6cda5ec634934d2 Mon Sep 17 00:00:00 2001 From: Patrice Billaut Date: Sun, 15 Sep 2024 18:40:13 +0200 Subject: [PATCH 1/2] refactor: use decimal instead of float --- Cargo.lock | 448 ++++++++++++++++++++++++++++++- Cargo.toml | 2 + benches/benchmarks/processing.rs | 17 +- src/account.rs | 165 ++++++------ src/account_activity.rs | 5 +- src/processor.rs | 23 +- src/processors/csv/reader.rs | 13 +- src/processors/csv/writer.rs | 7 +- src/transaction.rs | 7 +- 9 files changed, 565 insertions(+), 122 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b49c8e1..3e22fa0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,17 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "ahash" +version = "0.7.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "891477e0c6a8957309ee5c45a6368af3ae14bb510732d2684ffa19af310920f9" +dependencies = [ + "getrandom", + "once_cell", + "version_check", +] + [[package]] name = "aho-corasick" version = "1.1.3" @@ -72,18 +83,94 @@ version = "1.0.89" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "86fdf8605db99b54d3cd748a44c6d04df638eb5dafb219b135d0149bd0db01f6" +[[package]] +name = "arrayvec" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" + [[package]] name = "autocfg" version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c4b4d0bd25bd0b74681c0ad21497610ce1b7c91b1022cd21c80c6fbdd9476b0" +[[package]] +name = "bitvec" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bc2832c24239b0141d5674bb9174f9d68a8b5b3f2753311927c172ca46f7e9c" +dependencies = [ + "funty", + "radium", + "tap", + "wyz", +] + +[[package]] +name = "borsh" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6362ed55def622cddc70a4746a68554d7b687713770de539e59a739b249f8ed" +dependencies = [ + "borsh-derive", + "cfg_aliases", +] + +[[package]] +name = "borsh-derive" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3ef8005764f53cd4dca619f5bf64cafd4664dada50ece25e4d81de54c80cc0b" +dependencies = [ + "once_cell", + "proc-macro-crate", + "proc-macro2", + "quote", + "syn 2.0.77", + "syn_derive", +] + [[package]] name = "bumpalo" version = "3.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" +[[package]] +name = "bytecheck" +version = "0.6.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23cdc57ce23ac53c931e88a43d06d070a6fd142f2617be5855eb75efc9beb1c2" +dependencies = [ + "bytecheck_derive", + "ptr_meta", + "simdutf8", +] + +[[package]] +name = "bytecheck_derive" +version = "0.6.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3db406d29fbcd95542e92559bed4d8ad92636d1ca8b3b72ede10b4bcc010e659" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + +[[package]] +name = "byteorder" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" + +[[package]] +name = "bytes" +version = "1.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8318a53db07bb3f8dca91a600466bdb3f2eaadeedfdbcf02e1accbad9271ba50" + [[package]] name = "cast" version = "0.3.0" @@ -96,6 +183,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "cfg_aliases" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" + [[package]] name = "ciborium" version = "0.2.2" @@ -154,7 +247,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn", + "syn 2.0.77", ] [[package]] @@ -284,6 +377,29 @@ dependencies = [ "log", ] +[[package]] +name = "equivalent" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" + +[[package]] +name = "funty" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" + +[[package]] +name = "getrandom" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4567c8db10ae91089c99af84c68c38da3ec2f087c3f82960bcdbf3656b6f4d7" +dependencies = [ + "cfg-if", + "libc", + "wasi", +] + [[package]] name = "half" version = "2.4.1" @@ -294,6 +410,21 @@ dependencies = [ "crunchy", ] +[[package]] +name = "hashbrown" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +dependencies = [ + "ahash", +] + +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" + [[package]] name = "heck" version = "0.5.0" @@ -306,6 +437,16 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fbf6a919d6cf397374f7dfeeea91d974c7c0a7221d0d0f4f20d859d329e53fcc" +[[package]] +name = "indexmap" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68b900aa2f7301e21c36462b170ee99994de34dff39a4a6a528e80e7376d07e5" +dependencies = [ + "equivalent", + "hashbrown 0.14.5", +] + [[package]] name = "is-terminal" version = "0.4.13" @@ -425,6 +566,8 @@ dependencies = [ "clap", "criterion", "csv", + "rust_decimal", + "rust_decimal_macros", "serde", "test-log", "thiserror", @@ -466,6 +609,47 @@ dependencies = [ "plotters-backend", ] +[[package]] +name = "ppv-lite86" +version = "0.2.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77957b295656769bb8ad2b6a6b09d897d94f05c41b069aede1fcdaa675eaea04" +dependencies = [ + "zerocopy", +] + +[[package]] +name = "proc-macro-crate" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ecf48c7ca261d60b74ab1a7b20da18bede46776b2e55535cb958eb595c5fa7b" +dependencies = [ + "toml_edit", +] + +[[package]] +name = "proc-macro-error" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" +dependencies = [ + "proc-macro-error-attr", + "proc-macro2", + "quote", + "version_check", +] + +[[package]] +name = "proc-macro-error-attr" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" +dependencies = [ + "proc-macro2", + "quote", + "version_check", +] + [[package]] name = "proc-macro2" version = "1.0.86" @@ -475,6 +659,26 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "ptr_meta" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0738ccf7ea06b608c10564b31debd4f5bc5e197fc8bfe088f68ae5ce81e7a4f1" +dependencies = [ + "ptr_meta_derive", +] + +[[package]] +name = "ptr_meta_derive" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16b845dbfca988fa33db069c0e230574d15a3088f147a87b64c7589eb662c9ac" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "quote" version = "1.0.37" @@ -484,6 +688,42 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "radium" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc33ff2d4973d518d823d61aa239014831e521c75da58e3df4840d3f47749d09" + +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom", +] + [[package]] name = "rayon" version = "1.10.0" @@ -548,6 +788,70 @@ version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a66a03ae7c801facd77a29370b4faec201768915ac14a721ba36f20bc9c209b" +[[package]] +name = "rend" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "71fe3824f5629716b1589be05dacd749f6aa084c87e00e016714a8cdfccc997c" +dependencies = [ + "bytecheck", +] + +[[package]] +name = "rkyv" +version = "0.7.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9008cd6385b9e161d8229e1f6549dd23c3d022f132a2ea37ac3a10ac4935779b" +dependencies = [ + "bitvec", + "bytecheck", + "bytes", + "hashbrown 0.12.3", + "ptr_meta", + "rend", + "rkyv_derive", + "seahash", + "tinyvec", + "uuid", +] + +[[package]] +name = "rkyv_derive" +version = "0.7.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "503d1d27590a2b0a3a4ca4c94755aa2875657196ecbf401a42eff41d7de532c0" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + +[[package]] +name = "rust_decimal" +version = "1.36.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b082d80e3e3cc52b2ed634388d436fe1f4de6af5786cc2de9ba9737527bdf555" +dependencies = [ + "arrayvec", + "borsh", + "bytes", + "num-traits", + "rand", + "rkyv", + "serde", + "serde_json", +] + +[[package]] +name = "rust_decimal_macros" +version = "1.36.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da991f231869f34268415a49724c6578e740ad697ba0999199d6f22b3949332c" +dependencies = [ + "quote", + "rust_decimal", +] + [[package]] name = "ryu" version = "1.0.18" @@ -563,6 +867,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "seahash" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c107b6f4780854c8b126e228ea8869f4d7b71260f962fefb57b996b8959ba6b" + [[package]] name = "serde" version = "1.0.210" @@ -580,7 +890,7 @@ checksum = "243902eda00fad750862fc144cea25caca5e20d615af0a81bee94ca738f1df1f" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.77", ] [[package]] @@ -604,6 +914,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "simdutf8" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f27f6278552951f1f2b8cf9da965d10969b2efdea95a6ec47987ab46edfe263a" + [[package]] name = "smallvec" version = "1.13.2" @@ -616,6 +932,17 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "syn" +version = "1.0.109" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "syn" version = "2.0.77" @@ -627,6 +954,24 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn_derive" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1329189c02ff984e9736652b1631330da25eaa6bc639089ed4915d25446cbe7b" +dependencies = [ + "proc-macro-error", + "proc-macro2", + "quote", + "syn 2.0.77", +] + +[[package]] +name = "tap" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" + [[package]] name = "test-log" version = "0.2.16" @@ -646,7 +991,7 @@ checksum = "5999e24eaa32083191ba4e425deb75cdf25efefabe5aaccb7446dd0d4122a3f5" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.77", ] [[package]] @@ -666,7 +1011,7 @@ checksum = "a4558b58466b9ad7ca0f102865eccc95938dca1a74a856f2b57b6629050da261" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.77", ] [[package]] @@ -689,6 +1034,38 @@ dependencies = [ "serde_json", ] +[[package]] +name = "tinyvec" +version = "1.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "445e881f4f6d382d5f27c034e25eb92edd7c784ceab92a0937db7f2e9471b938" +dependencies = [ + "tinyvec_macros", +] + +[[package]] +name = "tinyvec_macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" + +[[package]] +name = "toml_datetime" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0dd7358ecb8fc2f8d014bf86f6f638ce72ba252a2c3a2572f2a795f1d23efb41" + +[[package]] +name = "toml_edit" +version = "0.22.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "583c44c02ad26b0c3f3066fe629275e50627026c51ac2e595cca4c230ce1ce1d" +dependencies = [ + "indexmap", + "toml_datetime", + "winnow", +] + [[package]] name = "tracing" version = "0.1.40" @@ -708,7 +1085,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.77", ] [[package]] @@ -762,12 +1139,24 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" +[[package]] +name = "uuid" +version = "1.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "81dfa00651efa65069b0b6b651f4aaa31ba9e3c3ce0137aaad053604ee7e0314" + [[package]] name = "valuable" version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "830b7e5d4d90034032940e4ace0d9a9a057e7a45cd94e6c007832e39edb82f6d" +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + [[package]] name = "walkdir" version = "2.5.0" @@ -778,6 +1167,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "wasi" +version = "0.11.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" + [[package]] name = "wasm-bindgen" version = "0.2.93" @@ -800,7 +1195,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 2.0.77", "wasm-bindgen-shared", ] @@ -822,7 +1217,7 @@ checksum = "afc340c74d9005395cf9dd098506f7f44e38f2b4a21c6aaacf9a105ea5e1e836" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.77", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -955,3 +1350,42 @@ name = "windows_x86_64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + +[[package]] +name = "winnow" +version = "0.6.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68a9bda4691f099d435ad181000724da8e5899daa10713c2d432552b9ccd3a6f" +dependencies = [ + "memchr", +] + +[[package]] +name = "wyz" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f360fc0b24296329c78fda852a1e9ae82de9cf7b27dae4b7f62f118f77b9ed" +dependencies = [ + "tap", +] + +[[package]] +name = "zerocopy" +version = "0.7.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b9b4fd18abc82b8136838da5d50bae7bdea537c574d8dc1a34ed098d6c166f0" +dependencies = [ + "byteorder", + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.7.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa4f8080344d4671fb4e831a13ad1e68092748387dfc4f55e356242fae12ce3e" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.77", +] diff --git a/Cargo.toml b/Cargo.toml index 1135d72..da3b054 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,8 @@ categories = ["finance", "parser-implementations"] anyhow = "1.0" clap = { version = "4.5.17", features = ["derive"] } csv = "1.3.0" +rust_decimal = "1.36" +rust_decimal_macros = "1.36" serde = { version = "1.0.210", features = ["derive"] } thiserror = "1.0.63" tracing = "0.1.40" diff --git a/benches/benchmarks/processing.rs b/benches/benchmarks/processing.rs index 6b8b7d0..64e66e8 100644 --- a/benches/benchmarks/processing.rs +++ b/benches/benchmarks/processing.rs @@ -4,27 +4,28 @@ use payment_processor::account_activity::AccountActivity; use payment_processor::processor::process_activities; use payment_processor::transaction::TransactionID; use payment_processor::ClientID; +use rust_decimal_macros::dec; fn bench_process_transactions(c: &mut Criterion) { let client_id_1 = ClientID(1); let client_id_2 = ClientID(2); let transactions = vec![ - AccountActivity::deposit(TransactionID(1), client_id_1, 100.0), + AccountActivity::deposit(TransactionID(1), client_id_1, dec!(100.0)), AccountActivity::dispute(TransactionID(1), client_id_1), - AccountActivity::deposit(TransactionID(2), client_id_1, 100.0), - AccountActivity::withdrawal(TransactionID(3), client_id_1, 50.0), + AccountActivity::deposit(TransactionID(2), client_id_1, dec!(100.0)), + AccountActivity::withdrawal(TransactionID(3), client_id_1, dec!(50.0)), AccountActivity::resolve(TransactionID(1), client_id_1), AccountActivity::dispute(TransactionID(2), client_id_1), AccountActivity::chargeback(TransactionID(2), client_id_1), - AccountActivity::withdrawal(TransactionID(4), client_id_1, 100.0), - AccountActivity::deposit(TransactionID(1), client_id_2, 100.0), + AccountActivity::withdrawal(TransactionID(4), client_id_1, dec!(100.0)), + AccountActivity::deposit(TransactionID(1), client_id_2, dec!(100.0)), AccountActivity::dispute(TransactionID(1), client_id_2), - AccountActivity::deposit(TransactionID(2), client_id_2, 100.0), - AccountActivity::withdrawal(TransactionID(3), client_id_2, 50.0), + AccountActivity::deposit(TransactionID(2), client_id_2, dec!(100.0)), + AccountActivity::withdrawal(TransactionID(3), client_id_2, dec!(50.0)), AccountActivity::resolve(TransactionID(1), client_id_2), AccountActivity::dispute(TransactionID(2), client_id_2), AccountActivity::chargeback(TransactionID(2), client_id_2), - AccountActivity::withdrawal(TransactionID(4), client_id_2, 100.0), + AccountActivity::withdrawal(TransactionID(4), client_id_2, dec!(100.0)), ]; c.bench_function("process_activities [dispute process]", move |b| { diff --git a/src/account.rs b/src/account.rs index 5182ab0..7a1a0b2 100644 --- a/src/account.rs +++ b/src/account.rs @@ -3,14 +3,16 @@ use crate::account_activity::AccountActivityError::{FailedDisputeCase, FailedTra use crate::account_activity::AccountActivityResult; use crate::transaction::{Transaction, TransactionID}; use crate::ClientID; +use rust_decimal::Decimal; +use rust_decimal_macros::dec; use std::collections::hash_map::Entry; use std::collections::HashMap; /// A valid amount is neither negative (including -0.0), infinite, [subnormal] nor NaN. /// /// [subnormal]: https://en.wikipedia.org/wiki/Denormal_number -fn is_valid_amount(amount: f32) -> bool { - amount == 0.0 || amount.is_normal() && amount.is_sign_positive() +fn is_valid_amount(amount: Decimal) -> bool { + amount.is_sign_positive() } #[derive(Debug, PartialEq, serde::Serialize)] @@ -18,16 +20,16 @@ pub struct Account { #[serde(rename = "client")] client_id: ClientID, - available: f32, + available: Decimal, - held: f32, + held: Decimal, - total: f32, + total: Decimal, locked: bool, #[serde(skip)] - dispute_cases: HashMap, + dispute_cases: HashMap, #[serde(skip)] transaction_record: HashMap, @@ -37,9 +39,9 @@ impl Account { pub fn new(client_id: ClientID) -> Self { Self { client_id, - held: 0.0, - total: 0.0, - available: 0.0, + held: dec!(0.0), + total: dec!(0.0), + available: dec!(0.0), locked: false, dispute_cases: HashMap::new(), transaction_record: HashMap::new(), @@ -51,17 +53,17 @@ impl Account { } /// Returns the total funds that are available. - pub fn available(&self) -> f32 { + pub fn available(&self) -> Decimal { self.available } /// Returns the total funds that disputed. - pub fn held(&self) -> f32 { + pub fn held(&self) -> Decimal { self.held } /// Returns the total funds that are [`available`](Self::available) or [`held`](Self::held). - pub fn total(&self) -> f32 { + pub fn total(&self) -> Decimal { self.total } @@ -74,7 +76,7 @@ impl Account { self.locked = true; } - fn deposit(&mut self, amount: f32) -> AccountActivityResult<()> { + fn deposit(&mut self, amount: Decimal) -> AccountActivityResult<()> { if !is_valid_amount(amount) { return Err(InvalidTransaction("deposit amount must be a positive number".into())); } @@ -83,7 +85,7 @@ impl Account { Ok(()) } - fn withdraw(&mut self, amount: f32) -> AccountActivityResult<()> { + fn withdraw(&mut self, amount: Decimal) -> AccountActivityResult<()> { if !is_valid_amount(amount) { Err(InvalidTransaction("withdrawal amount must be a positive number".into())) } else if amount > self.available { @@ -95,7 +97,7 @@ impl Account { } } - fn hold(&mut self, amount: f32) -> AccountActivityResult<()> { + fn hold(&mut self, amount: Decimal) -> AccountActivityResult<()> { if !is_valid_amount(amount) { Err(InvalidTransaction("hold amount must be a positive number".into())) } else { @@ -105,7 +107,7 @@ impl Account { } } - fn release(&mut self, amount: f32) -> AccountActivityResult<()> { + fn release(&mut self, amount: Decimal) -> AccountActivityResult<()> { if !is_valid_amount(amount) { Err(InvalidTransaction("release amount must be a positive number".into())) } else { @@ -115,7 +117,7 @@ impl Account { } } - fn charge(&mut self, amount: f32) -> AccountActivityResult<()> { + fn charge(&mut self, amount: Decimal) -> AccountActivityResult<()> { if !is_valid_amount(amount) { Err(InvalidTransaction("chargeback amount must be a positive number".into())) } else { @@ -208,6 +210,8 @@ impl Account { pub mod test_utils { use super::Account; use crate::ClientID; + use rust_decimal::Decimal; + use rust_decimal_macros::dec; use std::collections::HashMap; pub enum LockStatus { @@ -218,9 +222,9 @@ pub mod test_utils { impl Account { pub fn with_values( client_id: ClientID, - available: f32, - held: f32, - total: f32, + available: Decimal, + held: Decimal, + total: Decimal, lock_status: LockStatus, ) -> Self { Self { @@ -242,9 +246,9 @@ pub mod test_utils { fn default() -> Self { Self::with_values( ClientID::default(), - 0.0, - 0.0, - 0.0, + dec!(0.0), + dec!(0.0), + dec!(0.0), LockStatus::Unlocked, ) } @@ -257,6 +261,7 @@ mod test_account_activities { use crate::account_activity::AccountActivity; use crate::transaction::TransactionID; use crate::ClientID; + use rust_decimal_macros::dec; #[test] fn transactions_with_same_id_are_only_processed_once() { @@ -264,12 +269,12 @@ mod test_account_activities { let deposit_a = AccountActivity::deposit( transaction_id, ClientID::default(), - 100.0, + dec!(100.0), ); let deposit_b = AccountActivity::deposit( transaction_id, ClientID::default(), - 200.0, + dec!(200.0), ); let mut account = Account::default(); @@ -278,9 +283,9 @@ mod test_account_activities { let result = account.transaction(deposit_b); assert!(result.is_err(), "Expected second deposit transaction to fail"); - assert_eq!(account.available(), 100.0); - assert_eq!(account.held(), 0.0); - assert_eq!(account.total(), 100.0); + assert_eq!(account.available(), dec!(100.0)); + assert_eq!(account.held(), dec!(0.0)); + assert_eq!(account.total(), dec!(100.0)); } #[test] @@ -288,7 +293,7 @@ mod test_account_activities { let deposit = AccountActivity::deposit( TransactionID::default(), ClientID::default(), - 100.0, + dec!(100.0), ); let dispute = AccountActivity::dispute( deposit.transaction_id(), @@ -300,9 +305,9 @@ mod test_account_activities { let result = account.transaction(dispute); assert!(result.is_ok(), "Expected dispute to succeed: {:?}: {:?}", dispute, result); - assert_eq!(account.available(), 0.0); - assert_eq!(account.held(), 100.0); - assert_eq!(account.total(), 100.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.held(), dec!(100.0)); + assert_eq!(account.total(), dec!(100.0)); } #[test] @@ -325,12 +330,12 @@ mod test_account_activities { let deposit_a = AccountActivity::deposit( TransactionID(0), client_id, - 50.0, + dec!(50.0), ); let deposit_b = AccountActivity::deposit( TransactionID(1), client_id, - 50.0, + dec!(50.0), ); let dispute_a = AccountActivity::dispute( @@ -354,9 +359,9 @@ mod test_account_activities { assert!(result.is_ok(), "Expected dispute to succeed: {:?}: {:?}", dispute_b, result); - assert_eq!(account.available(), 0.0); - assert_eq!(account.held(), 100.0); - assert_eq!(account.total(), 100.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.held(), dec!(100.0)); + assert_eq!(account.total(), dec!(100.0)); } #[test] @@ -364,7 +369,7 @@ mod test_account_activities { let deposit = AccountActivity::deposit( TransactionID::default(), ClientID::default(), - 50.0, + dec!(50.0), ); let dispute = AccountActivity::dispute( deposit.transaction_id(), @@ -385,7 +390,7 @@ mod test_account_activities { let deposit = AccountActivity::deposit( TransactionID::default(), ClientID::default(), - 50.0, + dec!(50.0), ); let dispute = AccountActivity::dispute( deposit.transaction_id(), @@ -402,8 +407,8 @@ mod test_account_activities { let result = account.transaction(resolve); assert!(result.is_ok(), "Expected resolution to succeed: {:?}: {:?}", resolve, result); - assert_eq!(account.available(), 50.0); - assert_eq!(account.held(), 0.0); + assert_eq!(account.available(), dec!(50.0)); + assert_eq!(account.held(), dec!(0.0)); } #[test] @@ -422,7 +427,7 @@ mod test_account_activities { let deposit = AccountActivity::deposit( TransactionID::default(), ClientID::default(), - 50.0, + dec!(50.0), ); let dispute = AccountActivity::dispute( deposit.transaction_id(), @@ -440,8 +445,8 @@ mod test_account_activities { let result = account.transaction(chargeback); assert!(result.is_ok(), "Expected chargeback to succeed: {:?}: {:?}", chargeback, result); - assert_eq!(account.held(), 0.0); - assert_eq!(account.total(), 0.0); + assert_eq!(account.held(), dec!(0.0)); + assert_eq!(account.total(), dec!(0.0)); } #[test] @@ -460,7 +465,7 @@ mod test_account_activities { let deposit = AccountActivity::deposit( TransactionID::default(), ClientID::default(), - 50.0, + dec!(50.0), ); let dispute = AccountActivity::dispute( deposit.transaction_id(), @@ -484,10 +489,11 @@ mod test_account_activities { #[cfg(test)] mod test_accounting { use super::*; + use rust_decimal_macros::dec; #[test] fn deposit_affects_funds() { - let amount = 100.0; + let amount = dec!(100.0); let mut account = Account::default(); @@ -501,55 +507,53 @@ mod test_accounting { fn deposit_with_invalid_value_fails() { let mut account = Account::default(); - let lower_than_min = 1.0e-40_f32; - let invalid_values = [-1.0, lower_than_min, f32::NAN, f32::INFINITY]; + let invalid_values = [dec!(-1.0)]; for invalid_value in invalid_values { let result = account.deposit(invalid_value); assert!(result.is_err(), "Expected deposit with invalid value to fail: {:?}", invalid_value); - assert_eq!(account.available(), 0.0); - assert_eq!(account.total(), 0.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.total(), dec!(0.0)); } } #[test] fn withdrawal_affects_funds() { - let amount = 100.0; + let amount = dec!(100.0); let mut account = Account::default(); account.deposit(amount).expect("Test setup: deposit failed"); let result = account.withdraw(amount); assert!(result.is_ok(), "Expected withdrawal to succeed: {:?}", result); - assert_eq!(account.available(), 0.0); - assert_eq!(account.total(), 0.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.total(), dec!(0.0)); } #[test] fn withdraw_with_invalid_value_fails() { let mut account = Account::default(); - let lower_than_min = 1.0e-40_f32; - let invalid_values = [-1.0, lower_than_min, f32::NAN, f32::INFINITY]; + let invalid_values = [dec!(-1.0)]; for invalid_value in invalid_values { let result = account.withdraw(invalid_value); assert!(result.is_err(), "Expected withdrawal with invalid value to fail: {:?}", invalid_value); - assert_eq!(account.available(), 0.0); - assert_eq!(account.total(), 0.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.total(), dec!(0.0)); } } #[test] fn withdraw_with_insufficient_funds_fails() { - let available_funds = 100.0; + let available_funds = dec!(100.0); let mut account = Account::default(); account.deposit(available_funds).expect("Test setup: deposit failed"); - let result = account.withdraw(available_funds + 0.1); + let result = account.withdraw(available_funds + dec!(0.1)); assert!(result.is_err(), "Expected withdrawal exceeding available funds to fail"); assert_eq!(account.available(), available_funds); assert_eq!(account.total(), available_funds); @@ -557,14 +561,14 @@ mod test_accounting { #[test] fn hold_affects_funds() { - let amount = 100.0; + let amount = dec!(100.0); let mut account = Account::default(); account.deposit(amount).expect("Test setup: deposit failed"); let result = account.hold(amount); assert!(result.is_ok(), "Expected hold to succeed: {:?}", result); - assert_eq!(account.available(), 0.0); + assert_eq!(account.available(), dec!(0.0)); assert_eq!(account.held(), amount); assert_eq!(account.total(), amount); } @@ -573,22 +577,21 @@ mod test_accounting { fn hold_with_invalid_value_fails() { let mut account = Account::default(); - let lower_than_min = 1.0e-40_f32; - let invalid_values = [-1.0, lower_than_min, f32::NAN, f32::INFINITY]; + let invalid_values = [dec!(-1.0)]; for invalid_value in invalid_values { let result = account.hold(invalid_value); assert!(result.is_err(), "Expected hold with invalid value to fail: {:?}", invalid_value); - assert_eq!(account.available(), 0.0); - assert_eq!(account.held(), 0.0); - assert_eq!(account.total(), 0.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.held(), dec!(0.0)); + assert_eq!(account.total(), dec!(0.0)); } } #[test] fn release_affects_funds() { - let amount = 100.0; + let amount = dec!(100.0); let mut account = Account::default(); account.deposit(amount).expect("Test setup: deposit failed"); @@ -597,7 +600,7 @@ mod test_accounting { let result = account.release(amount); assert!(result.is_ok(), "Expected release to succeed: {:?}", result); assert_eq!(account.available(), amount); - assert_eq!(account.held(), 0.0); + assert_eq!(account.held(), dec!(0.0)); assert_eq!(account.total(), amount); } @@ -605,22 +608,21 @@ mod test_accounting { fn release_with_invalid_value_fails() { let mut account = Account::default(); - let lower_than_min = 1.0e-40_f32; - let invalid_values = [-1.0, lower_than_min, f32::NAN, f32::INFINITY]; + let invalid_values = [dec!(-1.0)]; for invalid_value in invalid_values { let result = account.release(invalid_value); assert!(result.is_err(), "Expected release with invalid value to fail: {:?}", invalid_value); - assert_eq!(account.available(), 0.0); - assert_eq!(account.held(), 0.0); - assert_eq!(account.total(), 0.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.held(), dec!(0.0)); + assert_eq!(account.total(), dec!(0.0)); } } #[test] fn charge_back_affects_funds() { - let amount = 100.0; + let amount = dec!(100.0); let mut account = Account::default(); account.deposit(amount).expect("Test setup: deposit failed"); @@ -628,25 +630,24 @@ mod test_accounting { let result = account.charge(amount); assert!(result.is_ok(), "Expected charge back to succeed: {:?}", result); - assert_eq!(account.available(), 0.0); - assert_eq!(account.held(), 0.0); - assert_eq!(account.total(), 0.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.held(), dec!(0.0)); + assert_eq!(account.total(), dec!(0.0)); } #[test] fn charge_back_with_invalid_value_fails() { let mut account = Account::default(); - let lower_than_min = 1.0e-40_f32; - let invalid_values = [-1.0, lower_than_min, f32::NAN, f32::INFINITY]; + let invalid_values = [dec!(-1.0)]; for invalid_value in invalid_values { let result = account.charge(invalid_value); assert!(result.is_err(), "Expected charge_back with invalid value to fail: {:?}", invalid_value); - assert_eq!(account.available(), 0.0); - assert_eq!(account.held(), 0.0); - assert_eq!(account.total(), 0.0); + assert_eq!(account.available(), dec!(0.0)); + assert_eq!(account.held(), dec!(0.0)); + assert_eq!(account.total(), dec!(0.0)); } } } diff --git a/src/account_activity.rs b/src/account_activity.rs index 63d7b47..dce44aa 100644 --- a/src/account_activity.rs +++ b/src/account_activity.rs @@ -1,6 +1,7 @@ use crate::dispute::DisputeCase; use crate::transaction::{Transaction, TransactionID}; use crate::ClientID; +use rust_decimal::Decimal; use std::fmt::{Display, Formatter}; use thiserror::Error; @@ -78,11 +79,11 @@ pub enum AccountActivity { } impl AccountActivity { - pub fn deposit(transaction_id: TransactionID, client_id: ClientID, amount: f32) -> Self { + pub fn deposit(transaction_id: TransactionID, client_id: ClientID, amount: Decimal) -> Self { Self::Deposit(Transaction::new(transaction_id, client_id, amount)) } - pub fn withdrawal(transaction_id: TransactionID, client_id: ClientID, amount: f32) -> Self { + pub fn withdrawal(transaction_id: TransactionID, client_id: ClientID, amount: Decimal) -> Self { Self::Withdrawal(Transaction::new(transaction_id, client_id, amount)) } diff --git a/src/processor.rs b/src/processor.rs index 7b5f72a..6614b2b 100644 --- a/src/processor.rs +++ b/src/processor.rs @@ -59,6 +59,7 @@ mod tests { use crate::processor::tests::DummyError::ParseError; use crate::transaction::TransactionID; use crate::ClientID; + use rust_decimal_macros::dec; use thiserror::Error; #[derive(Error, Debug, Clone)] @@ -91,22 +92,22 @@ mod tests { Ok(AccountActivity::deposit( TransactionID(1), ClientID::default(), - 10.0 + dec!(10.0) )), // The next record couldn't be parsed Err(ParseError), Ok(AccountActivity::withdrawal( TransactionID(2), ClientID::default(), - 5.0 + dec!(5.0) )) ], expected: vec![ Account::with_values( ClientID::default(), - 5.0, - 0.0, - 5.0, + dec!(5.0), + dec!(0.0), + dec!(5.0), LockStatus::Unlocked ) ], @@ -122,26 +123,26 @@ mod tests { Ok(AccountActivity::deposit( TransactionID(1), ClientID::default(), - 10.0 + dec!(10.0) )), // The next activity should cause an insufficient funds error Ok(AccountActivity::withdrawal( TransactionID::default(), ClientID::default(), - 15.0 + dec!(15.0) )), Ok(AccountActivity::withdrawal( TransactionID(2), ClientID::default(), - 10.0 + dec!(10.0) )), ], expected: vec![ Account::with_values( ClientID::default(), - 0.0, - 0.0, - 0.0, + dec!(0.0), + dec!(0.0), + dec!(0.0), LockStatus::Unlocked ) ], diff --git a/src/processors/csv/reader.rs b/src/processors/csv/reader.rs index 980de63..800a79a 100644 --- a/src/processors/csv/reader.rs +++ b/src/processors/csv/reader.rs @@ -82,6 +82,7 @@ mod tests { use crate::processors::csv::CsvProcessorResult; use crate::transaction::TransactionID; use crate::ClientID; + use rust_decimal_macros::dec; struct TestCase<'a> { input: Vec<&'a str>, @@ -138,13 +139,13 @@ mod tests { "withdrawal, 1, 3, 100.0", ], expected: vec![ - AccountActivity::deposit(TransactionID(1), ClientID(1), 100.0), - AccountActivity::deposit(TransactionID(2), ClientID(2), 100.0), + AccountActivity::deposit(TransactionID(1), ClientID(1), dec!(100.0)), + AccountActivity::deposit(TransactionID(2), ClientID(2), dec!(100.0)), AccountActivity::dispute(TransactionID(1), ClientID(1)), AccountActivity::dispute(TransactionID(2), ClientID(2)), AccountActivity::resolve(TransactionID(1), ClientID(1)), AccountActivity::chargeback(TransactionID(2), ClientID(2)), - AccountActivity::withdrawal(TransactionID(3), ClientID(1), 100.0), + AccountActivity::withdrawal(TransactionID(3), ClientID(1), dec!(100.0)), ], }) } @@ -159,9 +160,9 @@ mod tests { "withdrawal, 1, 3, 4.2", ], expected: vec![ - AccountActivity::deposit(TransactionID(1), ClientID(1), 8.0), - AccountActivity::withdrawal(TransactionID(2), ClientID(1), 1.5), - AccountActivity::withdrawal(TransactionID(3), ClientID(1), 4.2), + AccountActivity::deposit(TransactionID(1), ClientID(1), dec!(8.0)), + AccountActivity::withdrawal(TransactionID(2), ClientID(1), dec!(1.5)), + AccountActivity::withdrawal(TransactionID(3), ClientID(1), dec!(4.2)), ], }) } diff --git a/src/processors/csv/writer.rs b/src/processors/csv/writer.rs index af41191..03f4f59 100644 --- a/src/processors/csv/writer.rs +++ b/src/processors/csv/writer.rs @@ -34,14 +34,15 @@ mod tests { use super::*; use crate::account::{test_utils::LockStatus, Account}; use crate::ClientID; + use rust_decimal_macros::dec; #[test] fn serialize_account() { let account = Account::with_values( ClientID(101), - 10.0, - 20.0, - 30.0, + dec!(10.0), + dec!(20.0), + dec!(30.0), LockStatus::Locked, ); let expected = [ diff --git a/src/transaction.rs b/src/transaction.rs index bc821fd..2fded7a 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -1,4 +1,5 @@ use crate::ClientID; +use rust_decimal::Decimal; use std::fmt::{Display, Formatter}; /// A globally unique transaction ID. @@ -22,11 +23,11 @@ pub struct Transaction { #[serde(rename = "client")] client_id: ClientID, - amount: f32, + amount: Decimal, } impl Transaction { - pub fn new(id: TransactionID, client_id: ClientID, amount: f32) -> Self { + pub fn new(id: TransactionID, client_id: ClientID, amount: Decimal) -> Self { Self { id, client_id, amount } } @@ -38,7 +39,7 @@ impl Transaction { self.client_id } - pub fn amount(&self) -> f32 { + pub fn amount(&self) -> Decimal { self.amount } } \ No newline at end of file From 7577cc7bd2d3f4a2b300fa6b4e5edb4dc0cdc939 Mon Sep 17 00:00:00 2001 From: Patrice Billaut Date: Sun, 15 Sep 2024 19:31:55 +0200 Subject: [PATCH 2/2] refactor: replace sanity checks --- src/account.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/account.rs b/src/account.rs index 7a1a0b2..7cbc766 100644 --- a/src/account.rs +++ b/src/account.rs @@ -8,13 +8,6 @@ use rust_decimal_macros::dec; use std::collections::hash_map::Entry; use std::collections::HashMap; -/// A valid amount is neither negative (including -0.0), infinite, [subnormal] nor NaN. -/// -/// [subnormal]: https://en.wikipedia.org/wiki/Denormal_number -fn is_valid_amount(amount: Decimal) -> bool { - amount.is_sign_positive() -} - #[derive(Debug, PartialEq, serde::Serialize)] pub struct Account { #[serde(rename = "client")] @@ -77,16 +70,17 @@ impl Account { } fn deposit(&mut self, amount: Decimal) -> AccountActivityResult<()> { - if !is_valid_amount(amount) { - return Err(InvalidTransaction("deposit amount must be a positive number".into())); + if amount.is_sign_negative() { + Err(InvalidTransaction("deposit amount must be a positive number".into())) + } else { + self.available += amount; + self.total += amount; + Ok(()) } - self.available += amount; - self.total += amount; - Ok(()) } fn withdraw(&mut self, amount: Decimal) -> AccountActivityResult<()> { - if !is_valid_amount(amount) { + if amount.is_sign_negative() { Err(InvalidTransaction("withdrawal amount must be a positive number".into())) } else if amount > self.available { Err(FailedTransaction("withdrawal failed because of insufficient funds".into())) @@ -98,7 +92,7 @@ impl Account { } fn hold(&mut self, amount: Decimal) -> AccountActivityResult<()> { - if !is_valid_amount(amount) { + if amount.is_sign_negative() { Err(InvalidTransaction("hold amount must be a positive number".into())) } else { self.available -= amount; @@ -108,7 +102,7 @@ impl Account { } fn release(&mut self, amount: Decimal) -> AccountActivityResult<()> { - if !is_valid_amount(amount) { + if amount.is_sign_negative() { Err(InvalidTransaction("release amount must be a positive number".into())) } else { self.held -= amount; @@ -118,7 +112,7 @@ impl Account { } fn charge(&mut self, amount: Decimal) -> AccountActivityResult<()> { - if !is_valid_amount(amount) { + if amount.is_sign_negative() { Err(InvalidTransaction("chargeback amount must be a positive number".into())) } else { self.held -= amount;