Bookkeeping library crate

Yo, I made this open source bookkeeping crate. It's my first crate.

Most of my experience is from web development in JavaScript and TypeScript.

Would appreciate all the feedback.

I’ve only skimmed through it and it looks interesting. One area I have som feedback on is error handling.

I noticed immediately is that there is some panicking going on that I think you should try to avoid and return Result instead. Libraries should try to avoid panics if they can.

One example is in Book::insert_move where these are conditions where the library will panic:

  • transaction_index out of bounds.
  • move_index out of bounds.
  • Some of debit_account_key and credit_account_key are not in the book.
  • debit_account_key and credit_account_key are equal.
  • Some unit keys in the sum are not in the book.

This function should IMHO return a Result<(), BookError> where BookError is an error type you define which communicates what error condition occurred.

If you want to stick with panicking methods you can provide a Book::try_insert_move which returns a Result instead of panicking.

Take a look at thiserror for some help creating error types for your library. I can also recommend this article on error handling: https://blog.burntsushi.net/rust-error-handling/, especially from the “ Advice for library writers” paragraph and down.

1 Like

Thank you for your feedback, @cfsamson.

I added an answer to the FAQ section of the readme that should explain my view on this. Quoted here:

This API can panic in a bunch of places. I don't like that. I don't feel safe. How about returning Results, instead?

Results and errors are for when a function might fail regardless of usage. In this crate, panics would only occur on wrong usage. Having this crate return Results would complicate the API and — worse — would give the impression that function calls could fail regardless of usage. I'd like the user to be confidant that with correct usage the API is safe.

If that’s the strategy you want to take regarding errors, you might want to leverage the type system to turn some of those panics into compile errors. For instance, you could have the AccountKeys hold some kind of reference to their owning Book, and expose account-related operations as AccountKey methods. That would make it impossible for the programmer to accidentally use an account with the wrong book.

Some of those errors, though, might plausibly come from a user’s data entry error. (In particular, selecting the same account for both sides of a transaction.) That kind of error is usually best presented as a Result so that the program can present the user with an opportunity to correct their mistake instead of crashing.

Thank you for the feedback, @2e71828.

I would love that, but I can't quite imagine such an API. I can imagine only bank_key.remove_account() and bank_key.set_metadata() but what about inserting a move? Inserting a move requires two indexes (one of the transaction, one of the move) and two account keys.

Some changes have been made and the documentation (introduction) has been re-written. It now includes some explanations regarding design decisions. If anyone would like to review or review again, that would be great. Make sure to read both the crate-level doc and the introduction. Some criticisms that I have received are:

  • Too many generics make it complicated to understand and use.
  • The scope is too narrow, resulting in practically no value.

The two are kind of intertwined, because one of the possible ways of widening the scope is replacing some generics with concrete types (something I don't wish to do, because I like unopinionated libraries).

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.