0

Say I write a function which expects the input to be A or B. The input may be C, which is technically valid but should be impossible if the logic of the rest of the program is working correctly.

Should I:

  1. Add an if-else clause to handle all the possible inputs and throw an exception / return null if necessary.

  2. Assume the input can not be C and only handle the rest.

Which of those 2 variants are better? The first variant let us catch the error as soon as possible, but leads to very cluttered and hard to read code. The second variant leads to cleaner code, but if the rest of the program is working incorrectly, the bug may propagate far from its origin.

This is to address the internal logic of a program, not the user input, which we obviously should be able to handle all the possible inputs.

New contributor
SeifeUndSalz is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
4

Defensive programming

The input may be C, which is technically valid but should be impossible if the logic of the rest of the program is working correctly.

The keyword here is if. This is where we delve into the realm of defensive programming:

Defensive programming is a form of defensive design intended to ensure the continuing function of a piece of software under unforeseen circumstances. Defensive programming practices are often used where high availability, safety, or security is needed.

Most importantly, take note of the last sentence. Defensive programming is useful only when defending against mistakes is important. If this application is a small personal tool you use, it matters little.

Assuming that this application is important enough to ensure that only a valid value is entered, then defensive programming applies.


Implementing defenses

Say I write a function which expects the input to be A or B.

Right off the bat, enum came to mind. When you're dealing with a restricted list of options, and there's no existing primitive that covers only those restricted options, then an enum allows you to create a closed list.

public enum MyOptions { A, B }

This is, in my opinion, the cleanest approach, but it is also not a perfect defense. Enums are really just ints under the hood, and you can force any int value into it, e.g.:

public enum MyOptions { A = 0, B = 1 }

MyOptions valid   = (MyOptions) 1;
MyOptions invalid = (MyOptions) 2;  // No compiler or runtime error!

This brings us back to considering how much you need to defend. If abusing the enum causes a security breach, then you shouldn't just rely on enums to ensure security. If abusing the enum yields no benefits to the abuser, then it's acceptable to keep it at that.

As for your suggestions:

  1. Assume the input can not be C and only handle the rest.

Assuming there is no big risk involved (e.g. security breaches), this is the preferred approach.

That being said, I do still urge you to implement the enum, as it will help future developers (consumers of your code) to realize which values are expected/allowed.

  1. Add an if-else clause to handle all the possible inputs and throw an exception / return null if necessary.

Regardless of using the enum or not, when there is a risk involved with not checking it (e.g. security breach), then the if (or similar validation structure) becomes the desired approach.

  • Thanks for your answer. I like it when the answer gives me some keyword ("defensive programming") so I can google more about the topic. Enum is a good solution if the space of possible values is small. But sometimes it's not the case (for example if it is assumed that the argument should not be greater than some value) or I just want to refactor the code by moving some repeated logic into a function and don't want to change anything in the old cold. So I guess offensive programming with some assertion statement is the way here, because security is no concern in this case. – SeifeUndSalz Dec 3 at 10:46
  • It stays risky. How have you checked your conditions? if only A or B are valid and you checked for : somecondition <> B, did you really mean not B or did you mean: somecondition = A. What happens when you introduce a valid C? – Pieter B Dec 3 at 10:47
  • It's hard for me to think of an example right now. Here is one situation (may be contrived): If I write a function that generates a list of prime number, then another function which takes a prime number >2 and do something with it, but its argument is int). If my prime number function is working correctly, then I should not get any even number. Should I explicitly state this assumption in the code I'm writing? If this function is just a helper function and used once and twice (only for my self), should I still catch that even case, in case my prime generating function is incorrect? – SeifeUndSalz Dec 3 at 11:05
  • 1
    @PieterB: Validation between A and B isn't the focus of the question though. In general, positive assertions tend to be safer (foo == MyOptions.A) but there's no universally correct solution. – Flater Dec 3 at 14:17
  • 1
    "Enums are really just ints under the hood, and you can force any int value into it" - This is language specific. There are many languages with more robust enums. – 8bittree Dec 3 at 20:23
1

Assuming you are writing in a language that is OO first rather than functional first.


Run time checks

Hard to read code? Why? You can start the method by expressing that certain values are not valid. That should make it easier to read/understand.

Validation is a different concern anyway, you should know where it starts and where it ends, refactor your code so that is easy to see, thus improving readability. If the code to validate the input is actually complicated enough to make it hard to read, that is a clue that you should be extracting the validation to another method.

I understand there are some situations where it is non-obvious how do to that without a compromise in efficiency. For example, validating a list could mean to iterate over it. And then iterate over it again to actually use it... Well, if you have a solution such as Linq or similar, you should be able to write the validation code separate from the use of the list, while iterating over it only once, thanks to Linq being lazily evaluated.

You might also want to avoid duplicated validation in the internals of the code... you could use conditional compilations, debug assertions, code contracts, or similar solutions to make the extra validation only on development and testing environments.


Compile time checks

If you have a good code contracts solution, it would be validated on build.

Speaking of tools validating the code... can you get or write one? I'm thinking of a lint or similar, which could check preconditions on the code and be added to build integration. I know such tool is not always cost effective, however if your situation is specific enough and important enough, then such tool would be of value.

However, it is better to express validations as part of the type system, so they are checked at compile time without the need of additional tools.

You could – for example – have a type T, and a type ValidatedT, that can only be initialized with valid values of T. Then your internal code will only accept ValidatedT (of course, with a better name), making sure that the validation already happened, and regardless of how many method the ValidatedT value is passed down, the validation only happened once. This should also improve readability, as you could clearly see where do you have T and where do you have ValidatedT. Not only that, but you could have ValidatedT for different situations...

I mean, if I say the type string, what is it? A username? a product description? what? However, if I say the type Username, then you know what it is in the system, and you know it is a valid value.

Oh, by the way, you can still get the benefits of easier to read code, although without the compile time checking, if you use type aliases.

  • Subtyping is a good solution in some case. Debug assertion and code contracts are good. I can always disable debug assertions. Code contracts are also good if they introduce no runtime overhead in real build. Like, I do not want the overhead of validation that my finite group is abelian if the order of the group is 40 for example ( because mathematically all groups of order 40 are abelian up to an isomorphism), if my group generating function is working correctly. That type of complicated relationship. – SeifeUndSalz Dec 3 at 11:24
  • @SeifeUndSalz I would argue you use different tools for different situations. About the code contracts, it depends how you express them in code. Some solution have you express them in comments, for example. Just make sure to put the checker in build/continuous integration. I'm not sure I understand the case of the abelian groups, yet it sounds to me that it should be a type. And for checking if your group generation is working correctly, you would use test automation, ideally unit tests. – Theraot Dec 3 at 11:44
  • @SeifeUndSalz I mean, baring a language such as Agda or Irdis where you do proof automation. Your best tools are going to be types and tests. See Ideology. – Theraot Dec 3 at 11:47
  • I see, thanks for your answers! – SeifeUndSalz Dec 3 at 11:50
0

One thing to consider is that if you start building checks to stop valid but unexpected input (C), you essentially coupling the function to the application. In other words, you are making the function less likely to be reusable in another context. If you look at what's going on in a typical application, there are many parts of it that can accept input that you will never be providing to it.

For example, you might have a limit on the length of postal codes which you store in a string. The String's constructor allows many more characters as well as characters that aren't valid in postal codes.

Should you then define a new String class that only allows the valid characters? You could but that's not a very common solution. Instead, we build checks in the code that creates the postal code string, perhaps in a PostalCode class that is composed of a string (and maybe some other things.)

If this function is already inherently coupled to this application as it is designed, it's probably not something to worry about. But if this function is useful on it's own, you might want to avoid building such assumptions into it. Even within an application, as it grows and is refined over time, you might find that this function could be reused elsewhere. If the constraints are built into it but don't apply in the new context, you will then need to address moving them somewhere else first.

Your Answer

SeifeUndSalz is a new contributor. Be nice, and check out our Code of Conduct.

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.