Legitimate Error Messages

[raconzor]

Quoted from another message: Going through and putting legitimate error messages instead of bare asserts is probably a good idea. Seems to me that we should define an InvalidInputsException (or something like that) and make a point to mostly raise those, so calling programs can easily distinguish between those and stuff breaking. Also note that i18n-ing the error messages in those exceptions so they can be displayed directly would probably make sense.

[cynyr]

I started some work on this for stats.Stats and allowed the passing of a locale using the same names wow uses currently. See my fork. Where would the best place to catch these exceptions be? or are we just going to require what ever runs this engine to catch them?

[raconzor]

The intent is for the caller to catch them since on the calculator side we have no responsibility for specifics of display.

[Aldriana]

If someone wanted to work up a quick command-line interface for testing purposes (or whatever) we'd want to handle them there, but in general the idea is that we're defining an interface, and part of that interface is "we generate these types of acceptions when you feed us garbage". What the caller does with those is up to them.

As an aside, cynyr, it looks like you branched off the main library quite a while ago and haven't pulled fresh changed in a while. I'd recommend that you (and everyone working on this) update semiregularly as it makes the eventual merges much less painful.

[Aldriana]

As an additional note, I suspect we want to define an InvalidInputException (or something like that) that all our errors extend from. That way, people can easily do a single catch-all exception handling for all errors we generate, or can break them down by type. Mentally designing this: I'd probably add a new directory "core" and then in core.exceptions define InvalidInputException, and then in each existing file define a subclass for errors from that file - i.e., in objects.talents define InvalidTalentsException(exceptions.InvalidInputException), and in rogue_talents define InvalidRogueTalentsException(talents.InvalidTalentsException). By keeping a good hierarchy, it makes it easy for people to classify and distinguish our exception types.

As a further aside, this feels like it wants to wait until after we get i18n set up, as pretty much every single error message we generate in this fashion should be i18ned.

[cynyr]

I like the hierarchy idea, and it is good to know that you can catch a base exception and all children will also be caught as well.

I'll hold off from doing too much work until i18n stuff is working, although I do have some very messy support for it. I use a dictionary with keys for locales matching the wow ones at the moment.

Maybe a link to http://help.github.com/forking/ would be a good idea for the readme.

[Aldriana]

Added a base exception and a sample usage. We still need to go through and replace all the asserts (outside of tests, anyway) with error messages extending from InvalidInputException, however.

[Aldriana]

I don't think we really need an issue to track this anymore; we have the framework in place, people just need to remember to add sensible error messages as they add checks and stuff moving forward. Closing.