4

I am writing a class whose __init__ uses either an id OR a slug argument but not both. I'd like to verify that the arguments are as expected. Is it proper, and good-practice to use an assert for the specific purpose of verifying an assumption about arguments, or should I be raising an exception if the arguments aren't as expected?

e.g.,

def __init__(self, id=None, slug=None):
    assert((id or slug) and not (id and slug))
B Robster
  • 40,605
  • 21
  • 89
  • 122
  • assert will raise an exception if it fails ... you can add a comma and then a message that will be attached to the exception eg `assert 1==2,"1 is not equal to 2"` – Joran Beasley Nov 23 '12 at 19:04
  • Assertions are removed when optimizing. See this related question's answer: http://stackoverflow.com/a/1838411/303748 – vicvicvic Nov 23 '12 at 19:07
  • Joran, yes, i realize this. Good tip on the message, though, I'll add one. – B Robster Nov 23 '12 at 19:14
  • 1
    I have to ask...if you can accept exactly one of `id` or `slug`, why is this not `__init__(self, id_or_slug)`? Then you can only pass one or the other. – cHao Nov 23 '12 at 19:24
  • cHao, You make a good point. I hadn't thought of that, but I feel like it could go either way. If its id_or_slug, I'll have to inspect the argument's value to figure out whether it's an id or a slug because the code to do the database lookup is different depending on which one it is. But given your suggestion, I'll try it that way and see how I like it. It seems to provide a cleaner interface, and eliminates the need for convoluted input verification or extra documentation. In truth, I was more curious about the question's topic itself rather than how best to write my code, but this helps. – B Robster Nov 23 '12 at 19:27
  • Could I suggest this equivalent statement? `assert(bool(id) != bool(slug))`. Sorry, non-minimized logic bothers me a little bit since I took a class where we spent a whole month working on that. – Lanaru Nov 23 '12 at 19:57
  • You can suggest it, but I won't use it. I get it, but it doesn't convey what I'm asserting particularly well(i.e., its less readable). I took a class where we spent a whole semester studying compilers, and minimizing logic is something they excel at. "Readability counts." -- The Zen of Python – B Robster Nov 23 '12 at 20:01
  • @BenRoberts I definitely agree with you that your version is more readable, albeit still not crystal clear at first glance. By the way, if you add @ and then the name of the user you are replying to at the beginning of your comment, it automatically notifies that user that you've made a reply. – Lanaru Nov 23 '12 at 21:02
  • possible duplicate of [Best practice for Python Assert](http://stackoverflow.com/questions/944592/best-practice-for-python-assert) – Mp0int Nov 24 '12 at 11:35
  • Closed? Come on guys. Is it forbidden to discuss "best practices" on SO? Because its definitely not a duplicate of the thing linked to by FallenAngel (which, incidentally, discsusses best practices). I feel like this is a legitimate, constructive question which was answered and in which I learned something quite valuable. Any of you guys feel free to contact me offline and can suggest how I could improve the question to reopen or explain to me how I'm off base. – B Robster Nov 24 '12 at 18:06
  • @Lanaru Thanks for the @ tip. (Don't know how i'd never picked up on that before.) I had a change of heart, and went with your minimization because if the test evaluates to True, I raise a ValueError with a message that explains what the test is all about, minimizing the arguably lesser readability of the test itself. – B Robster Nov 24 '12 at 18:27

3 Answers3

11

The answer to the original question ("Is it bad practice to use assert to verify internal assumptions in python?") is decidedly no. That's the one thing everyone agrees assertions are good for. But what you describe in the question isn't validation of an internal assumption: It's validation of external inputs. A TypeError, a ValueError, another builtin exception (or possibly a custom exception, though one should be conservative with those) gives much more useful feedback to users of the API. What's worse, assertions may be (and, in some environments, usually are) removed completely, meaning your code will silently do the wrong thing.

B Robster
  • 40,605
  • 21
  • 89
  • 122
  • Good synopsis of the issue. However, I guess i'm getting hung up on the difference between an internal assumption and validation of "external" inputs. If only other code (in my own code base for the time being) could potentially trigger this by instantiating the class incorrectly (i.e., it couldn't be triggered by a user of the software), is that an internal assumption or an external input? – B Robster Nov 23 '12 at 19:11
  • 3
    @BenRoberts: As far as your function cares, it's external input. You don't want your functions making too many assumptions about each other. Validate just as you would if random users were going to be feeding it input. – cHao Nov 23 '12 at 19:16
  • ``ValueError`` is also a valuabe candidate and (in the context of the example in the question), the preferable choice. – Jonas Schäfer Nov 23 '12 at 19:18
  • @JonasWielicki I like it better than `RuntimeError` here, but I don't see it applying here. Maybe I interpret it differently, but for me that's indicating an invalid value, not the wrong number of values. –  Nov 23 '12 at 19:20
  • 1
    Yeah, it's difficult, I'd raise a value error though, because the error message I have in mind is somewhat *Exactly one of foo and bar must not be None*, which is ValueError'ish for me. For me, keyword arguments are not countable in the sense of a invalid-argument-count ``TypeError``. – Jonas Schäfer Nov 23 '12 at 19:22
  • @delnan (and anyone else) I edited my question a bit to make it clearer what I'm asking and am a bit bothered by it having been closed. Could you vote to reopen or let me know if u think the closure was legit? – B Robster Nov 24 '12 at 18:10
  • I'm voting to re-open, I think it's a legitimate and perfectly answerable question. Then again, I might be biased. –  Nov 24 '12 at 18:17
7

If this is about code sanity, as you describe, an assert is the proper thing to use. You should not use assert if this is about input validation. In particular, a user should not be able to trigger the assert by supplying junk to it. An assert must only be triggered due to a bug in the program.

edit: As I've just learned from the comments, Python also allows you to turn assertions off completely (meaning they aren't evaluated and thus can't fail). This would be the proper answer (aside from using a different language like assembly or C) to performance issues, should there be any.

Bas Wijnen
  • 1,288
  • 1
  • 8
  • 17
  • The second paragraph makes no sense to me. You need to check the condition anyway, and if you do that, an assertion is at least as fast, if not faster (if disabled at compile time). –  Nov 23 '12 at 19:11
  • 1
    @delnan: If you really need to check the condition every time, then you shouldn't be using assertions. They're a sanity check, not an input validation tool. – cHao Nov 23 '12 at 19:13
  • If you know your function is called with correct arguments, you do not need to check it. I agree that it is useful to check it anyway, but if performance is a real issue, people may want to omit the check. Then again, as I wrote, this is probably only an issue for C and assembly programmers. – Bas Wijnen Nov 23 '12 at 19:14
  • @cHao An assertion is checked every time execution passes through it (same goes for conditions). That's not what differentiates assertions from `if ...: raise ...`. Furthermore, that's not what the answer says: It says assertions are slower than the alternative (whatever that is, I have to guess here). Which I really can't see happening. –  Nov 23 '12 at 19:19
  • While this post does say the right thing, I feel a bit uncomfortable to upvote, as it is somehow feeling like a “yes, but” on a question which should be answered “no”. Would you add some sentences about the specific example given in the question? That'd make it a truly upvotable answer. – Jonas Schäfer Nov 23 '12 at 19:19
  • @delnan the difference is, in python and others, that assertions can be turned off for performance benefits, and thus the check is omitted. That's what really distinguishes them from ``if …: raise …``, besides semantics. – Jonas Schäfer Nov 23 '12 at 19:20
  • @delnan: Your very own answer says that assertions are not always checked. – cHao Nov 23 '12 at 19:21
  • @JonasWielicki I know that fully well (I've mentioned it three times already). That's how assertions can be *faster, not slower*. –  Nov 23 '12 at 19:21
  • @cHao Yes, I meant to write "provided it is not disabled by `-O`, in which case it's *free*". –  Nov 23 '12 at 19:22
  • I'm confused by your comment “An assertion is checked every time execution passes through it (same goes for conditions). That's[…]” then, sorry. Didn't want to enerve you. – Jonas Schäfer Nov 23 '12 at 19:23
  • 1
    @delnan: OK, now that i'm actually getting your beef with the post, i have to agree; the performance argument is kinda BS. (If assertions cost too much, then turn them off in production -- you should be able to run without them anyway.) If the second paragraph were omitted entirely, though, the rest is worth an upvote. – cHao Nov 23 '12 at 19:28
  • @jonas: The alternative would be to have no check at all. If the program is correct, the assertion could be turned off or omitted. I wasn't aware that Python had an option to turn the assertions off. That would be the way to go then if performance is such an issue. – Bas Wijnen Nov 23 '12 at 19:28
4

Assert raises AssertionError when it fails. The python standard library frequently raises ValueError when an argument isn't quite correct -- Or a TypeError when you call a function with the wrong number of arguments:

consider:

int("15f")     #ValueError
int("15",2,3)  #TypeError

The question you need to ask yourself (from an API standpoint) is which is most logical for your function to raise on bad input? Whatever you choose, make sure you document it well so your users can know how to handle the error should it arise.

mgilson
  • 300,191
  • 65
  • 633
  • 696