The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Test all input parameteres in method


I'm having a method like this

public int GetNextIndex(int pos, int length)

I call the method from a loop and my experience tells me that sometimes I don't get loop logic programmed correcly, and it might be that the method could be called with pos > length.
Of course this would be a mistake in my loop logic and should never happend.

How do I handle this risk? Here are some of my thoughts

if (index > length)
  throw new ArgumentOutOfRangeException("index larger than index");

Debug.Assert(index <= length);

Make a very good unit test for the method that does the looping and would test every possible case. Even though I can't see how I can do that.

Nothing at all. If the normal unit tests passes, then I'm fine.

For method 1 and 2, I should perhaps also check that length is > 0 or should I use uint instead of int?
Karsten Send private email
Saturday, January 05, 2008
I would write a unit test that tested the likely conditions, and the edge case conditions, but not all of them.
For example, some conditions to check would be:
1. index < pos
2. index = pos
3. index > pos
as well as combinations of index and pos equals to 0 and int.Max, but I won't test every possible combinations, only if I suspected that there was a specific combination that resulted in a bug, would I test that as well.
Miki Watts Send private email
Saturday, January 05, 2008
No need to pick only one option.  Regardless of what else you do with unit tests, etc., if the method can be called with bad parameters, and you're the one calling it (i.e. it's not for a distributable class library), include a Debug.Assert in there to make catching your own bugs easier.

The more I use Debug.Assert, the more valuable I find it, both for catching bugs and for documenting assumptions that later turn out to be bad assumptions.  You'll probably want to include *both* Debug.Assert and an exception; the exception is for when the program hits it, but the Debug.Assert is for *your* benefit while coding/testing/debugging.
Kyralessa Send private email
Saturday, January 05, 2008
Public methods should throw exceptions even in release mode. Private methods should use asserts that are stripped out in release mode.

At least that's what I've been taught.

In Java the assert statements are always there (not stripped) but you can toggle whether or not they get invoked using a load-time parameter.
Gili Send private email
Saturday, January 05, 2008
Here are the set of rules I tend to use:

1. If you can reasonably detect that the arguments passed to the function are invalid, you should definitely do so.

2. You should definitely have Assert() statements to validate the constraints of the arguments (pos < length, length > 0, whatever). Asserts can make it much easier to debug problems during development. They're also great documentation of that the function's constraints are.

3. If this function is a public interface, then you need to do something "sensible" in the case where client code calls it with bad arguments. Depending on your language/environment, this might be throwing an exception, returning an error code, or returning a "safe" value. In the case of searching, you might just return "not found" if someone tries to search of the end, for example.

4. If your language/environment is such that the Assert() statements get stripped out of a Release build, then you obviously can't depend on them to help you find bugs once the code's gone out to the QA department or your customers.

Keep that in mind - in many cases, you won't ever see the situations that violate the constraints you've set. You still have to write (and test) your error-recovery code, because the Asserts won't catch all the possible errors during development.

5. Last but not least, don't ever write code that crashes in cases where you could have trivially prevented the crash. Yeah, you "know" this routine is never going to be called with the wrong parameters, because you've looked at all the callers. What if someone else changes the code later?
Mark Bessey Send private email
Sunday, January 06, 2008
I like to put a bunch of assert() statements at the beginning of the function implementation.  The assert statements do things like validate the function parameters and verify the state of any global structures I need.

I like doing this because it makes any assumptions about the code explicit.  By asserting the hell out of everything at the start, it reduces the amount of error-checking I have to worry about later in the function.  If something is wrong, it bombs immediately, and I know where/how it bombed.

Now, I only do this for internally-used functions.  You wouldn't want to use asserts in a function for user input for example.  If the user makes a typo, your program shouldn't abort.  Asserts are for catching broken logic, not "soft" errors.  For things like that, it make more sense to throw an exception or return an error code.

Based on your description, it sounds like (1) the function is something that you call internally, and (2) if the wrong paremeter is passed in, your logic is broken.  To me, this sounds like the right spot to use an assert().

Disclaimer:  I come from the C world, so my perpective may not necessarily be helpful for you situation.
Myron A. Semack Send private email
Sunday, January 06, 2008
Agree on adding asserts as the protection scheme.

Furthermore, I would recommend that on that condition, you also gather some additional information for how that condition came to be so you can fix the underlying problems that may exist in the loop code itself.

You definitely want to protect the internal logic from running with invalid parameters, but you also want to know how invalid parameters got there to begin with.
Cory von Wallenstein Send private email
Monday, January 07, 2008

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
Powered by FogBugz