views:

365

answers:

4

After running Code Analysis in VS2010 beta (FxCop for previous versions) I'm getting the following warning:

In externally visible method 'Identity.Identity(WindowsIdentity)', validate parameter 'windowsIdentity' before using it.

The constructor is:

public Identity(WindowsIdentity windowsIdentity)
         : base(windowsIdentity.Token)
{
         init();
}

for a class defined as:

public class Identity : WindowsIdentity

My question is, how do I validate the windowsIdentity parameter? Should I validate it in the constructor, and throw an exception, or is there a better way to call this?

A: 

I believe FXCop reports this error here because it thinks that you could encounter a NullReferenceException by accessing windowsIdentity when calling the base class constructor.

One way to add a validation check for null would be to add a static private function to your class that that can check the WindowsIdentity parameter for null and take appropriate action:

private static WindowsIdentity ValidateIdentity( WindowsIdentity identity )
{
    if( identity == null )
        throw new ArgumentNullException( "identity" );
    // possibly some other validation checks here...

    return identity;        
}

public Identity(WindowsIdentity windowsIdentity)
    : base( ValidateIdentity( windowsIdentity ).Token )
{
     init();
}

Another approach would be to use the ternary operator to verify the parameter, as in:

public Identity(WindowsIdentity windowsIdentity)
    : base( windowsIdentity == null ? null : windowsIdentity.Token )
{
     init();
}

But, what you should really ask yourself is what would you do? If you're simply going to throw an exception, it may be ok to let the code stand as is, since it will already through a NullReferenceException if the argument is null.

LBushkin
+1  A: 

It's complaining because if you pass NULL as windowsIdentity, then when the constructor chains to the base class it will throw a null reference exception.

The best way to deal with it depends on your design. You could check it for null like this:

:base(windowsIdentity == null ? null : windowsIdentity.Token)

Or you could make another constructor in base class constructor that takes a WindowsIdentity as a parameter, and have that constructor do that part of the validation. Basically there are tons of ways to deal with it, just use what works best in your situation.

dan
+3  A: 

You can validate it in a static method:

public Identity(WindowsIdentity windowsIdentity)
         : base(GetToken(windowsIdentity))
{
         init();
}

static Token GetToken(WindowsIdentity ident)
{
    if(ident == null)
        throw new ArgumentNullException("ident");

    return ident.Token;
}

(I didn't bother to look for the type of WindowsIdentity.Token, but you get the idea)

Sander Rijken
This looks like the best approach, except I'm having it just return itself instead.
A: 

FX cop is telling you that the parameter can't be null, so if you really need it you should validate it somehow. Since you are using it in the constructor, you probably want a value different from null, so you should be validating it there for FX cop stop annoying you..

If you need a constructor with null, you should be having another constructor with no parameters.

If you are not using it or you are validating it in another point, you can skip the alert.

To avoid the problem with FXcop, you should be throwing ArgumentNullException.

gbianchi