5

I have next code:

private T CreateInstance<T>(object obj) // where T : ISomeInterface, class
{
    ...

    if (!typeof(T).IsAssignableFrom(obj.GetType())) { throw ..; }

    return (T)obj;
}

Can it be replaced with this:

T result = obj as T;

if (result == null) { throw ..; }

return result;

If not - why?

abatishchev
  • 98,240
  • 88
  • 296
  • 433

11 Answers11

6

What about if (!(bar is T)) { throw ..; }

Alternatively if you don't need your own exception message the simplest answer is just to do:

return (T)obj;

The reason if that if it's not castable an InvalidCastException will be thrown and the return ignored. Unless you're adding some more logic or a custom error message there's no need to do a check and throw your own exception.

Davy8
  • 30,868
  • 25
  • 115
  • 173
  • thanks, this way of programming ([Guard Clause Pattern](https://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html)) is better imo. – Ozkan Nov 08 '18 at 08:23
4

Another variant:

private T CreateInstance<T>(object obj) where T : ISomeInterface // as OP mentioned above
{
    ...

    T result = obj as T;
    if (result == null)
        { throw ..; }
    else 
       return result;
}
abatishchev
  • 98,240
  • 88
  • 296
  • 433
Denis Palnitsky
  • 18,267
  • 14
  • 46
  • 55
3

Yes you can use your as operator code there instead of the original code, so long as T is a reference type or nullable.

as is the recommended way of casting in C# (see item 3 of Effective C#, by Bill Wagner)

From system.type.isassignablefrom:

[returns] true if c and the current Type represent the same type, or if the current Type is in the inheritance hierarchy of c, or if the current Type is an interface that c implements, or if c is a generic type parameter and the current Type represents one of the constraints of c. false if none of these conditions are true, or if c is null.

From 7.10.11 of the C# spec:

In an operation of the form E as T, E must be an expression and T must be a reference type, a type parameter known to be a reference type, or a nullable type

So you can see that they do comparable checks.

Matt Ellen
  • 11,268
  • 4
  • 68
  • 90
2

Maybe this (less brackets, better readability)

if (obj is T)
{
    return (T)obj;
}
else
   throw new ...

EDITED by reduced number of brackets I originally meant inverted check: ie

if (obj is T)

instead of

if (!(obj is T))

so final version can be

if (obj is T)
{
    return (T)obj;
}

throw new ...

or

if (obj is T)
{
    return (T)obj;
}
else
{
   throw new ...
}
desco
  • 16,642
  • 1
  • 45
  • 56
  • More error prone, because it makes the slightest bit of sense to insert another line of code after a `throw` statement? – Joel Mueller Aug 03 '10 at 16:23
1

IsAssignableFrom used by this scene:

foreach (PropertyInfo property in GetType().GetProperties())
{
    if (typeof(SubPresenter).IsAssignableFrom(property.PropertyType))
    {//Do Sth.}
}
IvanYu
  • 11
  • 1
1

Just for the developers who like to play the numbers game (who doesn't!).

Below you'll find a performance comparison test for IsAssignableFrom vs. As. Of course this will only count if you have an instance.

The result of the test (one million attempts):

IsAssignableFrom: 146 ms elapsed

AsOperator: 7 ms elapsed

[TestMethod]
public void IsAssignableFromVsAsPerformanceTest()
{
    Stopwatch stopwatch = new Stopwatch();
    stopwatch.Start();

    int attempts = 1000000;
    string value = "This is a test";

    for (int attempt = 0; attempt < attempts; attempt++) {
        bool isConvertible = typeof(IConvertible).IsAssignableFrom(value.GetType());
    }

    stopwatch.Stop();
    Console.WriteLine("IsAssignableFrom: {0} ms elapsed", stopwatch.ElapsedMilliseconds);

    stopwatch.Restart();

    for (int attempt = 0; attempt < attempts; attempt++) {
        bool isConvertible = value as string != null;
    }

    stopwatch.Stop();
    Console.WriteLine("AsOperator: {0} ms elapsed", stopwatch.ElapsedMilliseconds);
}
Roel van Megen
  • 341
  • 3
  • 8
1

See this post

The second one is safe...because at the first one if obj is null you will get exception (obj.GetType() --> NullReferenceException).

When you place "is" and then "as" is cause performance issues..

Community
  • 1
  • 1
garik
  • 5,669
  • 5
  • 30
  • 42
  • I think he might need to add some Constraints but isn't that what he is trying to accomplish? If T can be this type... cast it? – Nix Aug 03 '10 at 13:05
1

The class constraint where T : class allows you to use the as T statement.

private T CreateInstance<T>(object obj) where T : class
{
    if (!(obj is T)) { throw new ArgumentException("..."); }
    return obj as T;
}

or

private T CreateInstance<T>(object obj)
{
    if (!(obj is T)) { throw new ArgumentException("..."); }
    return (T)obj;
}
Daniel Dyson
  • 13,192
  • 6
  • 42
  • 73
1

You're probably looking for the is keyword, with the syntax expression is type

Documentation describes it as performing the checks you want:

An is expression evaluates to true if both of the following conditions are met:

• expression is not null.

• expression can be cast to type. That is, a cast expression of the form (type)(expression) will complete without throwing an exception.

Edit However, if instead of just working out whether you can cast something before you try, the as keyword is probably your best solution as you describe in your post.

The following code would perform the same function though...

try
{
    T result = (T)obj;
    return result;
}
catch (InvalidCastException ex)
{
     // throw your own exception or deal with it in some other way.
}

Which method you prefer is up to you...

Fiona - myaccessible.website
  • 14,481
  • 16
  • 82
  • 117
-2

It may have been intended to handle cases where a conversion constructor would allow the operation, but apparently IsAssignableFrom doesn't handle that either. Don't see anything that can handle that. So I don't see how to check for cases like this:

class Program
{
  static void Main(string[] args)
  {
     B bValue = new B(123);
     Console.WriteLine(typeof(A).IsAssignableFrom(bValue.GetType()));
     //Console.WriteLine(bValue is A);
     //Console.WriteLine(bValue as A == null);
     A aValue = bValue;
     Console.WriteLine(aValue.ToString());
  }
}

class A
{
  string value;
  public A(string value)
  {
     this.value = value;
  }
  public override string ToString()
  {
     return value;
  }
}

class B
{
  int value;

  public B(int value)
  {
     this.value = value;
  }

  public static implicit operator A(B value)
  {
     return new A(value.value.ToString());
  }
}

In the end, I don't see any reason why you wouldn't want to use your version of the code, unless you want the code to throw an exception when obj is null. That's the only difference I can see. obj.GetType() will throw an null reference exception when obj is null instead of throwing the specified exception.

Edit: I see now your version of the code will not compile if T can be a value type, but the other suggested solution like "if (obj is T) return (T)obj;" will compile. So I see why your suggested alternative will not work, but I don't see why you couldn't use "is".

BlueMonkMN
  • 25,079
  • 9
  • 80
  • 146
  • I don't understand why people vote answers down without explaining the problem. What's the point? We don't learn anything from a down vote if nobody explains the problem. – BlueMonkMN Jul 28 '11 at 11:03
-3

Or even better because its easer to read true conditionals.

 if(obj is T){
    //Create instance. 
 }
 else{
    throw new InvalidArgumentException("Try Again");
 }
Nix
  • 57,072
  • 29
  • 149
  • 198
  • 3
    When you place "is" and then "as" is cause performance issues... http://stackoverflow.com/questions/686412/c-is-operator-performance/686431#686431 – garik Aug 03 '10 at 13:28
  • Really? Performance is not in question here? He is not asking what is the fastest way to do it. – Nix Aug 03 '10 at 14:23