I usually don't check if function input is invalid. Am I wrong?

A lot of times when reading source code I see something like this:

public void Foo(Bar bar)
{
    if (bar == null) return;

    bar.DoSomething();
}

I do not like this, but I appear to be in the wrong as this form of defensive programming is considered good. Is it though? For example, why is bar null to begin with? Isn't doing checks like this akin to applying a bandage to a problem rather than solving the real solution? Not only does it complicate functions with additional lines of code but it also prevents the programmer from seeing potential bugs.

Here's another example:

public void Foo(int x)
{
    int clientX = Math.Max(x, 0); // Ensures x is never negative
}

Others look at that and see defensive programming but I see future bugs when a programmer accidentally passes a negative value and the program suddenly breaks and no one knows why because this little bit of logic swallowed the potentially revealing exception.

Now, please do not confuse checking if user input is valid versus what I am asking here. Obviously user input should be checked. What I am asking only pertains to code that does not interact开发者_如何学Go with the user or his or her input.

this int clientX = Math.Max(x, 0); is NOT defensive programming - it is masquerading potential problems!

Defensive programming would be

if ( x < 0 )
     throw new Exception ( "whatever" ); // or return false or...

and defensive programming is absolutely recommended... you never know how this code will be called in the future so you make sure that it handles anything appropriately i.e. things it is unable to handle must be filtered out as early as possible and the caller must be "notified" (for example by a meaningful exception)...

You check for nulls because attempting to have a null object perform an operation will trigger an exception. "Nothing" cannot do "something." You code like this because you can never know 100% of the time what state your application will be in.

That being said, there are good and bad ways of coding against invalid states, and those examples you gave aren't exactly "good" ways, although it's hard to say when taken out of context.

A lot of time you might be building a component that will be used by different applications with input being supplied by different programmers with different coding styles, some may not perform thorough validation on data passed in to your component/method so defensive programming in this situation would be a good thing to catch this, regardless of what the user of the component does.

PS: one thing though, usually you would not just return from the method as you showed above, you would throw an appropriate Exception, maybe an InvalidArgumentException or something like that.

Probably you see this more often:

public void Foo(Bar bar)
{
    if (bar == null) throw new ArgumentNullException("bar");

    bar.DoSomething();
}

This way, if someone did supply a null value as parameter (maybe as a result of some other method), you don't see a NullReferenceException from "somewhere" in your code, but an exception that states the problem more clearly.

Simply returning on invalid input is akin to a silent try/catch in my opinion. I still validate data coming into my functions from other pieces of code, but always throw an appropriate exception when I encounter invalid input.

Something like,

int clientX = Math.Max(x, 0)

could have really bad effects in some cases, just assume if you get x negative because of fault in some other place of the program, this would cause error to propagate. I would rather suggest you log and to throw an exception (some special type specific to business logic) when undesirable situation occurs.