Skip to content

Commit bd316e0

Browse files
committed
Throw instead of panic
1 parent 114eb8a commit bd316e0

File tree

7 files changed

+91
-64
lines changed

7 files changed

+91
-64
lines changed

crates/core/src/logic/calendar_logic.rs

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -140,15 +140,20 @@ impl YearAndMonth {
140140
/// use klirr_core::prelude::*;
141141
/// let start = YearAndMonth::january(2025);
142142
/// let end = YearAndMonth::april(2025);
143-
/// assert_eq!(end.elapsed_months_since(start), 3);
143+
/// assert_eq!(end.elapsed_months_since(start).unwrap(), 3);
144144
/// ```
145145
///
146-
/// # Panics
147-
/// Panics if the `start` month is after the `end` month.
148-
pub fn elapsed_months_since(&self, start: impl Borrow<Self>) -> u16 {
146+
/// # Throws
147+
/// Throws an error if the `start` month is after the `end` month.
148+
pub fn elapsed_months_since(&self, start: impl Borrow<Self>) -> Result<u16> {
149149
let end = self;
150150
let start = start.borrow();
151-
assert!(start <= end, "Expected start <= end month");
151+
if start > end {
152+
return Err(Error::StartPeriodAfterEndPeriod {
153+
start: start.to_string(),
154+
end: end.to_string(),
155+
});
156+
}
152157
let start_year = **start.year();
153158
let start_month = **start.month() as u16;
154159
let end_year = **end.year();
@@ -159,7 +164,8 @@ impl YearAndMonth {
159164
let months_per_year = 12;
160165
let start_months = start_year * months_per_year + start_month;
161166
let end_months = end_year * months_per_year + end_month;
162-
end_months - start_months
167+
let months_elapsed = end_months - start_months;
168+
Ok(months_elapsed)
163169
}
164170
}
165171

@@ -192,7 +198,7 @@ impl IsPeriod for YearAndMonth {
192198
Granularity::Month
193199
}
194200

195-
fn elapsed_periods_since(&self, start: impl Borrow<Self>) -> u16 {
201+
fn elapsed_periods_since(&self, start: impl Borrow<Self>) -> Result<u16> {
196202
self.elapsed_months_since(start)
197203
}
198204

@@ -232,7 +238,7 @@ impl IsPeriod for YearAndMonth {
232238
/// &target_month,
233239
/// is_expenses,
234240
/// &months_off_record,
235-
/// );
241+
/// ).unwrap();
236242
///
237243
/// // The expected invoice number is calculated as follows:
238244
/// // - Offset is 100
@@ -248,13 +254,13 @@ pub fn calculate_invoice_number<Period: IsPeriod>(
248254
target_period: &Period,
249255
is_expenses: bool,
250256
record_of_periods_off: &RecordOfPeriodsOff<Period>,
251-
) -> InvoiceNumber {
252-
assert!(
253-
!record_of_periods_off.contains(offset.period()),
254-
"Record of periods off contains offset.period(): {:?} but it should not.",
255-
offset.period()
256-
);
257-
let periods_elapsed_since_offset = target_period.elapsed_periods_since(offset.period());
257+
) -> Result<InvoiceNumber> {
258+
if record_of_periods_off.contains(offset.period()) {
259+
return Err(Error::RecordsOffMustNotContainOffsetPeriod {
260+
offset_period: format!("{:?}", offset.period()),
261+
});
262+
}
263+
let periods_elapsed_since_offset = target_period.elapsed_periods_since(offset.period())?;
258264

259265
let mut periods_off_to_subtract = 0;
260266
for period_off in record_of_periods_off.iter() {
@@ -271,7 +277,7 @@ pub fn calculate_invoice_number<Period: IsPeriod>(
271277
// expenses the same month, the expense invoice number is always higher.
272278
invoice_number += 1;
273279
}
274-
InvoiceNumber::from(invoice_number)
280+
Ok(InvoiceNumber::from(invoice_number))
275281
}
276282

277283
/// Calculates the number of working days in a given month, excluding weekends.
@@ -589,7 +595,8 @@ mod tests {
589595
&YearAndMonth::from(target_period),
590596
is_expenses,
591597
information.record_of_periods_off(),
592-
);
598+
)
599+
.unwrap();
593600
assert_eq!(invoice_number, expected.into());
594601
}
595602

@@ -859,7 +866,7 @@ mod tests {
859866
fn test_elapsed_months_since_when_start_month_is_later_in_the_year_than_end_month() {
860867
let start = YearAndMonth::december(2024);
861868
let end = YearAndMonth::april(2025);
862-
assert_eq!(end.elapsed_months_since(start), 4);
869+
assert_eq!(end.elapsed_months_since(start).unwrap(), 4);
863870
assert!(start < end);
864871
}
865872

@@ -871,16 +878,14 @@ mod tests {
871878
}
872879

873880
#[test]
874-
#[should_panic]
875-
fn test_elapsed_months_since_panic() {
881+
fn elapsed_months_since_throws_when_start_month_is_later_than_end_month() {
876882
let start = YearAndMonth::april(2025);
877883
let end = YearAndMonth::march(2025);
878-
end.elapsed_months_since(start);
884+
assert!(end.elapsed_months_since(start).is_err());
879885
}
880886

881887
#[test]
882-
#[should_panic]
883-
fn test_calculate_invoice_number_panics_for_invalid_input() {
888+
fn test_calculate_invoice_number_throws_for_invalid_input() {
884889
let month = YearAndMonth::may(2025);
885890
let invoice_info = ProtoInvoiceInfo::builder()
886891
.offset(
@@ -893,12 +898,13 @@ mod tests {
893898
.purchase_order(PurchaseOrder::sample())
894899
.build();
895900

896-
let _ = calculate_invoice_number(
901+
let result = calculate_invoice_number(
897902
invoice_info.offset(),
898903
&YearAndMonth::december(2025),
899904
true,
900905
invoice_info.record_of_periods_off(),
901906
);
907+
assert!(result.is_err());
902908
}
903909

904910
#[test]

crates/core/src/models/data/data.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ impl<Period: IsPeriod> Data<Period> {
106106
&target_period,
107107
is_expenses,
108108
self.information().record_of_periods_off(),
109-
);
109+
)?;
110110
let is_expenses_str_or_empty = if is_expenses { "_expenses" } else { "" };
111111
let vendor_name = self.vendor.company_name().replace(' ', "_");
112112

crates/core/src/models/data/submodels/expenses_for_periods.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,38 +6,41 @@ use crate::{define_item_struct, prelude::*};
66
#[serde(transparent)]
77
pub(super) struct ExpensesForPeriods(Vec<Item>);
88

9+
/// Ephemeral type used such that expensed items sharing all fields except for quantity
10+
/// are considered the same, allowing us to merge them into a single item with the total quantity.
911
#[derive(Hash, Eq, PartialEq, Display, Clone, Debug, PartialOrd, Ord, Serialize, Deserialize)]
1012
struct QuantityIgnored;
11-
define_item_struct!(pub, Marker, QuantityIgnored);
13+
define_item_struct!(pub, ExpenseIdentifier, QuantityIgnored);
1214

1315
impl ExpensesForPeriods {
1416
/// Inserts a vector of items into the `ExpensesForPeriods`, merging items that are the same
1517
/// except for their quantity.
1618
pub(super) fn insert(&mut self, items: Vec<Item>) {
1719
self.0.extend(items);
1820

19-
let mut map = IndexMap::<Marker, Quantity>::new();
21+
let mut map = IndexMap::<ExpenseIdentifier, Quantity>::new();
2022
for item in &self.0 {
21-
let marker = Marker::builder()
23+
let identifier = ExpenseIdentifier::builder()
2224
.name(item.name().clone())
2325
.transaction_date(*item.transaction_date())
2426
.unit_price(*item.unit_price())
2527
.currency(*item.currency())
2628
.quantity(QuantityIgnored)
2729
.build();
2830

29-
map.entry(marker)
31+
map.entry(identifier)
3032
.and_modify(|q| *q += *item.quantity())
3133
.or_insert(*item.quantity());
3234
}
3335

34-
self.0.retain(|_| false);
35-
for (marker, quantity) in map {
36+
self.0.clear();
37+
38+
for (identifier, quantity) in map {
3639
let item = Item::builder()
37-
.name(marker.name().clone())
38-
.transaction_date(*marker.transaction_date())
39-
.unit_price(*marker.unit_price())
40-
.currency(*marker.currency())
40+
.name(identifier.name().clone())
41+
.transaction_date(*identifier.transaction_date())
42+
.unit_price(*identifier.unit_price())
43+
.currency(*identifier.currency())
4144
.quantity(quantity)
4245
.build();
4346
self.0.push(item);

crates/core/src/models/data/submodels/is_period.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ impl<T: Eq + PartialOrd + Hash + Clone + std::fmt::Debug + Into<PeriodAnno> + Tr
2020

2121
pub trait IsPeriod: PeriodMarker {
2222
fn max_granularity(&self) -> Granularity;
23-
fn elapsed_periods_since(&self, start: impl Borrow<Self>) -> u16;
23+
fn elapsed_periods_since(&self, start: impl Borrow<Self>) -> Result<u16>;
2424
fn to_date_end_of_period(&self) -> Date;
2525
fn year(&self) -> &Year;
2626
fn month(&self) -> &Month;

crates/core/src/models/data/submodels/period_anno.rs

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,18 @@ impl IsPeriod for PeriodAnno {
6767
}
6868

6969
/// Number of periods that have elapsed since the given start period.
70-
fn elapsed_periods_since(&self, start: impl Borrow<Self>) -> u16 {
70+
fn elapsed_periods_since(&self, start: impl Borrow<Self>) -> Result<u16> {
7171
match (self, start.borrow()) {
7272
(Self::YearAndMonth(lhs), Self::YearAndMonth(rhs)) => lhs.elapsed_periods_since(rhs),
7373
(Self::YearMonthAndFortnight(lhs), Self::YearMonthAndFortnight(rhs)) => {
7474
lhs.elapsed_periods_since(rhs)
7575
}
76-
_ => panic!("Cannot mix period kinds"),
76+
(Self::YearAndMonth(_), Self::YearMonthAndFortnight(_)) => {
77+
Err(Error::PeriodIsNotYearAndMonth)
78+
}
79+
(Self::YearMonthAndFortnight(_), Self::YearAndMonth(_)) => {
80+
Err(Error::PeriodIsNotYearMonthAndFortnight)
81+
}
7782
}
7883
}
7984

@@ -128,7 +133,7 @@ mod tests {
128133
fn test_elapsed_periods_since_year_and_month() {
129134
let early = Sut::YearAndMonth(YearAndMonth::december(2024));
130135
let late = Sut::YearAndMonth(YearAndMonth::february(2025));
131-
assert_eq!(late.elapsed_periods_since(&early), 2);
136+
assert_eq!(late.elapsed_periods_since(&early).unwrap(), 2);
132137
}
133138

134139
#[test]
@@ -147,7 +152,7 @@ mod tests {
147152
.half(MonthHalf::First)
148153
.build(),
149154
);
150-
assert_eq!(late.elapsed_periods_since(&early), 3); // whole of january (2) + half of december (1)
155+
assert_eq!(late.elapsed_periods_since(&early).unwrap(), 3); // whole of january (2) + half of december (1)
151156
}
152157

153158
#[test]
@@ -208,17 +213,17 @@ mod tests {
208213
}
209214

210215
#[test]
211-
#[should_panic(expected = "Cannot mix period kinds")]
212-
fn test_mix_ym_ymf() {
213-
let _ = Sut::YearAndMonth(YearAndMonth::sample())
216+
fn mix_ym_ymf_throws() {
217+
let result = Sut::YearAndMonth(YearAndMonth::sample())
214218
.elapsed_periods_since(Sut::YearMonthAndFortnight(YearMonthAndFortnight::sample()));
219+
assert!(result.is_err(), "Expected error when mixing period kinds");
215220
}
216221

217222
#[test]
218-
#[should_panic(expected = "Cannot mix period kinds")]
219-
fn test_mix_ymf_ym() {
220-
let _ = Sut::YearMonthAndFortnight(YearMonthAndFortnight::sample())
223+
fn mix_ymf_ym_throws() {
224+
let result = Sut::YearMonthAndFortnight(YearMonthAndFortnight::sample())
221225
.elapsed_periods_since(Sut::YearAndMonth(YearAndMonth::sample()));
226+
assert!(result.is_err(), "Expected error when mixing period kinds");
222227
}
223228

224229
#[test]

0 commit comments

Comments
 (0)