13

I am trying to convert from a String a a generic type. The generic type will be an Int32, Int64, Boolean, Double and so on ... I tried two approaches:

public static Boolean TryParse<T>(String source, out T value) {

  TypeConverter converter = TypeDescriptor.GetConverter(typeof(T));

  try {

    value = (T)converter.ConvertFromString(source);
    return true;

  } catch {

    value = default(T);
    return false;

  }

}

public static Boolean TryChangeType<T>(Object source, out T value) {

  try {

    Type type = Nullable.GetUnderlyingType(typeof(T)) ?? typeof(T);

    value = (T)Convert.ChangeType(source, type);
    return true;

  } catch {

    value = default(T);
    return false;

  }

}

The second one is more generic as it accepts an Object.

I am also considering passing an IFormatProvider in TryChangeType that would be used in Convert.ChangeType to resolve Culture issues and so on.

Do you consider the second approach better?

Any way I can improve my code?

Miguel Moura
  • 36,732
  • 85
  • 259
  • 481
  • Possible duplicate of [TypeConverter vs. Convert vs. TargetType.Parse](http://stackoverflow.com/questions/7010669/typeconverter-vs-convert-vs-targettype-parse) – Michael Freidgeim Mar 15 '17 at 21:20

3 Answers3

9

In the first example of yours you can get rid of the try catch block by calling CanConvertTo() and CanConvertFrom() beforehand.

public static bool TryParse<T>(string source, out T value)
{
    TypeConverter converter = TypeDescriptor.GetConverter(typeof (T));
    if (converter.CanConvertTo(typeof (T)) && converter.CanConvertFrom(typeof (string)))
    {
        value = (T)converter.ConvertFromString(source);
        return true;
    }
    else
    {
        value = default (T);
        return false;
    }
}

In the second example why not make it even more generic and pass in a generic type?

Convert only works if the type implements the interface IConvertible so you can check for that, on the other hand it stil doesn't ensure that the conversion will be possible.

        public static bool TryChangeType<T, TR>(T input, out TR output) where T : IConvertible
    {
        bool result = false;
        try
        {
            Type type = Nullable.GetUnderlyingType(typeof(TR));
            output = (TR)Convert.ChangeType(input, type);
            result = true;
        }
        catch(Exception)
        {
            output = default(TR);
        }
        return result;
    }

It would be nice to only catch the exceptions you know of:

            catch(InvalidCastException)
        {
            output = default(TR);
            //Conversion is not unsupported
        }
            catch(FormatException)
        {
            output = default(TR);
            //string input value was in incorrect format
        }
            catch(InvalidCastException)
        {
            output = default(TR);
            //Conversion is not unsupported
        }
            catch(OverflowException)
        {
            output = default(TR);
            //narrowing conversion between two numeric types results in loss of data
        }

This might not answer the question fully, but you were asking for possible improvements so I thought why not.

Bradley Uffner
  • 16,641
  • 3
  • 39
  • 76
pijemcolu
  • 2,257
  • 22
  • 36
  • Yes, that makes even more generic. Good suggestion. – Miguel Moura Jun 24 '16 at 11:36
  • I strongly recommend avoiding many catch blocks that do the exact same thing, consider using [`when`](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/when) instead. – jrh Oct 09 '18 at 14:05
  • Also regarding "only catch the exceptions you know of", IMO that's a judgement call, if you use `catch(Exception)` you are assuming that if the framework class throws an exception, it will be recoverable; if you catch only known exceptions, you are assuming that the entire framework has demolished itself irrecoverably (but has not used a [CSE exception type](https://stackoverflow.com/questions/7392783/list-of-exceptions-that-cant-be-caught-in-net)). I would argue that the former is more likely and [I have ran into it in the past](https://codereview.stackexchange.com/q/139474/99069). – jrh Oct 09 '18 at 14:13
  • ... essentially not catching all exceptions is saying "if the framework's documentation is lacking in any way (exceptions thrown are not enforced by the compiler), or there is any bug in the framework, my application should crash", whereas catching all exceptions is saying "if the framework fails in an unexpected manner that *isn't* marked as a corrupted state, I take responsibility for rolling back any changes because whatever happened in this block is undefined". Admittedly the handling of this "roll back" is nontrivial and must be done carefully. – jrh Oct 09 '18 at 14:18
2

The second one is applicable only for IConvertible types. If this is what you want, you might want to apply a constraint, too (ChangeType will throw an exception for non-convertible types anyway):

public static Boolean TryChangeType<T>(Object source, out T value)
    where T : IConvertible
{
    // ...
}

The first one is more general, TypeConverter is used when the .NET component model should be used. For example, in designers, type converters are used to convert the values from string in the property grid. But you should add a small additional check here, too:

if (!converter.CanConvertFrom(typeof(string)))
    return false;

Additionally, I would mention that you should use the ConvertFromInvariantString method if you do not want trouble with the different region settings (in case of floating point values, for example)...

György Kőszeg
  • 17,093
  • 6
  • 37
  • 65
  • About the ConvertFromInvariantString that is a good tip ... I was having problems with converting doubles with "." or "," ... But using the second option I could pass an IFormatProvider ... That solves that problem on the second example, right? – Miguel Moura Jun 24 '16 at 11:39
  • Yes, using a specified culture solves the problem, too. Btw, you can convert from any `object` source here, too. Just use the `CanConvertFrom(Type)` method. – György Kőszeg Jun 24 '16 at 11:43
0

In your first example TypeDescriptor.GetConverter(Type) can throw exceptions, so move that into the try block. Without trying them personally, out of your two approaches I like the second one.

This post shows an example of testing for convertibility with no try/catch and claims performance benefits.

Community
  • 1
  • 1
leetibbett
  • 843
  • 4
  • 16