schotime / NPoco

Simple microORM that maps the results of a query onto a POCO object. Project based on Schotime's branch of PetaPoco
Apache License 2.0
848 stars 302 forks source link

Throw more specific exception types #607

Open bryanboettcher opened 4 years ago

bryanboettcher commented 4 years ago

There are 37 "throw new Exception" in v5 -- these should be made to more specific exception types to communicate errors both programmatically and textually.

I can create a PR for this as well if you're open to it.

schotime commented 4 years ago

I'd be open to a PR that created an NPocoException and had a property with a type on it or something but not multiple exception types.

bryanboettcher commented 4 years ago

Well, my intent would be to use the framework exception types (IIRC there were some bits of the code that were like throw new Exception("parameter is null") -- and those make sense to be ArgumentNullException, for instance.

As for NPoco-specific exceptions, would you prefer a just the single NPocoException, or a base NPocoException with maybe 3 subtypes? I would agree that not every single exception spot needs a custom exception, but extending from a base NPocoException would allow consumers to catch Exception, catch NPocoException, catch NPocoConfigurationException, etc. I don't want to add too much further maintenance overhead, but at the same time C# has a very rich exception-handling system and it'd be a shame to waste it :)

schotime commented 4 years ago

Well, my intent would be to use the framework exception types (IIRC there were some bits of the code that were like throw new Exception("parameter is null") -- and those make sense to be ArgumentNullException, for instance.

Agreed

As for NPoco-specific exceptions, would you prefer a just the single NPocoException, or a base NPocoException with maybe 3 subtypes? I would agree that not every single exception spot needs a custom exception, but extending from a base NPocoException would allow consumers to catch Exception, catch NPocoException, catch NPocoConfigurationException, etc. I don't want to add too much further maintenance overhead, but at the same time C# has a very rich exception-handling system and it'd be a shame to waste it :)

Lets see where you might actually wanna catch those exceptions and then make the decision once we know where we stand then, remembering you can do when on exceptions now and check the properties if you wanna be specific.