umya-spreadsheet icon indicating copy to clipboard operation
umya-spreadsheet copied to clipboard

Proposal: Gracefully handling errors and panic reduction in the library.

Open agentjill opened this issue 11 months ago • 3 comments

@MathNya, For version3.0, I am proposing following

  1. Functions like (there are multiple, given only one as an example) get_sheet returns Option<&Worksheet>. The functions gives None when it cant get the sheet, but there is also an assert! to check if the sheet is properly deserialized. Therefore panics when it finds the sheet but is not deserialized. I find this rather clunky, if functions has multiple failure modes, then it is optimal to make the return type as Result rather than an Option. Better we just let them handle failure cases than panicing user's code.
  2. The code contains multiple instances of String type errors like "Not found.", "out of index.", "sheet not found.", "name duplicate.", "Index cannot be less than one.", "Non-standard range." etc. Rather than strings to signify the error, it would be wise to take advantage of types to classify and relay them to the user.(Also applicable for point-1 above)
  3. I understand that the most of the panic! are associated with the reading of Zip file, but there is ZipError in the codebase which we can use.

Kindly comment your thoughts on my request for improving function signatures and removal of unnecessary panics.

agentjill avatar Mar 08 '25 02:03 agentjill