12
\$\begingroup\$

Given a boxed integer (with a boxed short, int, ...) I want to convert it to an unboxed, nullable type instead. It should be possible to convert non-matching types (for example: boxed int to short?) Can you please comment on:

using System;
public class Test
{
 public static void Main()
 {
 int? x = Unbox<int>((short)42);
 short? y = Unbox<short>(42);
 }
 static T? Unbox<T>(object x) where T : struct
 {
 return typeof(T) == x.GetType() ? (T)x : (T)Convert.ChangeType(x, typeof(T));
 }
}
asked Jan 10, 2017 at 13:24
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to Code Review! Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers . \$\endgroup\$ Commented Jan 10, 2017 at 14:51

5 Answers 5

16
\$\begingroup\$

You should probably add IConvertible restriction on T, since it is used by Convert.ChangeType method.

Also this check: typeof(T) == x.GetType() looks like an overhead. Convert.ChangeType does nothing if types match. For example, check out Int32 sources:

int IConvertible.ToInt32(IFormatProvider provider) {
 return m_value;
}

So you might as well just check for null instead:

static T? Unbox<T>(object x) where T : struct, IConvertible
{
 if (x == null) return null;
 return (T)Convert.ChangeType(x, typeof(T));
}

Edit: as mentioned by sir t3chb0t, there are cases, where Convert.ChangeType can throw. So it might be a good idea to swallow exceptions inside your method and return null in those cases. But it also can be a very bad idea. It depends on use case, which you should carefully evaluate.

answered Jan 10, 2017 at 13:44
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Note though, that ChangeType could have taken an IConvertible but chose to take object instead. It does support non-IConvertible "conversions". (See also stackoverflow.com/questions/3847997/…) \$\endgroup\$ Commented Jan 10, 2017 at 14:38
  • 2
    \$\begingroup\$ should just make it return x == null ? null : (T)Convert.ChangeType(x, typeof(T)); one line it, or maybe better return x != null ? (T)Convert.ChangeType(x, typeof(T)) : null; \$\endgroup\$ Commented Jan 10, 2017 at 14:57
  • \$\begingroup\$ @user127866 great observation. So if types match, ChangeType does not even hit actual IConvertible implementation. That's pretty weird design choice on Microsoft's part. \$\endgroup\$ Commented Jan 11, 2017 at 7:34
15
\$\begingroup\$

I'm not sure if you understand what boxing means because you use an int in the example.

Boxing is the process of converting a value type to the type object or to any interface type implemented by this value type.

and

When the CLR boxes a value type, it wraps the value inside a System.Object and stores it on the managed heap.

and

Unboxing extracts the value type from the object.

This would be unboxing. First box the int as object and then unbox it again.

object value = (short)42;
int? x = (int?)(short)value;
short? y = (short?)value;

I find this method is unecessary. You're better off with normal casting then with this and its overhead.

The problem with ChangeType is that

The ChangeType(Object, Type) method can convert a nullable type to another type. However, it cannot convert another type to a value of a nullable type, even if conversionType is the underlying type of the Nullable.To perform the conversion, you can use a casting operator (in C#) or a conversion function (in Visual Basic). The following example illustrates the conversion to and from a nullable type.

So this won't work:

short? z = (short?)Convert.ChangeType(value, typeof(short?));

but you

want to convert it to an unboxed, nullable type instead.

this means that if you still want to use the Unbox method the correct version of it should able to actually accept the int? as a parameter and return the default(T) rather then a null:

static T Unbox<T>(object x)
{
 if (x == null) return default(T);
 var nullableType = Nullable.GetUnderlyingType(typeof(T));
 return (T)Convert.ChangeType(x, nullableType ?? typeof(T));
}

but this is still casting the result with (T) so we're back at the beginning... this method is unnecessary.


Oh, and there is one more thing. ChangeType will blow if you try to convert int to short that exceedes the short.

It'll also hide the fact that you can loose precision when converting incompatibile types.

Consider this:

object value = (double)12.3;
short x = (short)(double)value;
int y = Unbox<int>(value);

To get the x you need to explicitly and deliberately cast it to short and see and know that you are converting double and may loose precision.

Getting the y via Unbox silently removes the fractional part.

answered Jan 10, 2017 at 15:42
\$\endgroup\$
3
  • \$\begingroup\$ While your statements are true, I am sorry that I failed in making the question more clear. a) Unboxed takes objects, "you use an int" is not correct/missunderstanding. b1) I did not intend that ConvertType would handle null, this was a bug in my question (which the rules of this site prevented me to edit) Sorry. I just use a null check to handle nulls now. b2) Normal casting requires multiple methods. In this case I don't want normal casting because I don't know the type to cast to. A generic method is what I want. c) I should have said, that I am not concerned with overflows. I am sorry. \$\endgroup\$ Commented Jan 11, 2017 at 8:35
  • \$\begingroup\$ @user127866 thx for clarifying - next time you'll give more details and there won't by any misunderstandings ;-) \$\endgroup\$ Commented Jan 11, 2017 at 8:39
  • \$\begingroup\$ @user127866 out of curiosity, why don't you know the data type? \$\endgroup\$ Commented Jan 11, 2017 at 14:55
3
\$\begingroup\$

If you actually want to unbox the value and not simply convert it you can use the Expression API to build a function that does exactly that, in pseudo-code:

Func<object, TResult> = boxedValue => (TResult)(typeof(boxedValue))boxedValue;

To build the function you'll need a helper class to construct the delegate with the correct types:

static class UnboxerCache<TResult>
{
 private static readonly ConcurrentDictionary<Type, Func<object, TResult>> UnboxerLookup = new ConcurrentDictionary<Type, Func<object, TResult>>();
 public static Func<object, TResult> GetUnboxer(Type actualType)
 {
 return UnboxerLookup.GetOrAdd(actualType, CreateUnboxer);
 }
 private static Func<object, TResult> CreateUnboxer(Type actualType)
 {
 var boxedParameter = Expression.Parameter(typeof(object), "boxedValue");
 var unboxedValue = Expression.Unbox(boxedParameter, actualType);
 var convertedValue = Expression.Convert(unboxedValue, typeof(TResult));
 var unboxer = Expression.Lambda<Func<object, TResult>>(convertedValue, boxedParameter);
 return unboxer.Compile();
 }
}

Here we have a mapping from the actual type of the object to a delegate that can unbox and cast it to the result type. The construction of the delegate is pretty simple, convert the boxedValue to the actual type and then to the result type.

I would make the actual unboxer a separate class with that sole resposibility:

public static class Unbox
{
 public static TResult? To<TResult>(object boxedValue)
 where TResult : struct
 {
 if (boxedValue == null)
 {
 return null;
 }
 var actualType = boxedValue.GetType();
 var resultType = typeof(TResult);
 if (actualType == resultType)
 {
 return (TResult)boxedValue;
 }
 var unboxer = UnboxerCache<TResult>.GetUnboxer(actualType);
 return unboxer(boxedValue);
 }
 private static class UnboxerCache<TResult>
 {
 private static readonly ConcurrentDictionary<Type, Func<object, TResult>> UnboxerLookup = new ConcurrentDictionary<Type, Func<object, TResult>>();
 public static Func<object, TResult> GetUnboxer(Type actualType)
 {
 return UnboxerLookup.GetOrAdd(actualType, CreateUnboxer);
 }
 private static Func<object, TResult> CreateUnboxer(Type actualType)
 {
 var boxedParameter = Expression.Parameter(typeof(object), "boxedValue");
 var unboxedValue = Expression.Unbox(boxedParameter, actualType);
 var convertedValue = Expression.Convert(unboxedValue, typeof(TResult));
 var unboxer = Expression.Lambda<Func<object, TResult>>(convertedValue, boxedParameter);
 return unboxer.Compile();
 }
 }
}

Finally you should consider verifying that the type of the boxed value is a value type (boxedValue.GetType().IsValueType) and maybe raise an error if that isn't the case.

answered Jan 11, 2017 at 10:25
\$\endgroup\$
2
\$\begingroup\$

Unbox<T> is missing a null check. The call x.GetType() will fail if x = null.

static T? Unbox<T>(object x) where T : struct
{
 if (x == null)
 return null;
 return typeof(T) == x.GetType() ? (T)x : (T)Convert.ChangeType(x, typeof(T));
}
answered Jan 10, 2017 at 14:59
\$\endgroup\$
0
\$\begingroup\$

I've found this to work well:

if (valueType == typeof(int))
 field.Value =(int) Convert.ChangeType(reader.Value, valueType);
else 
 field.Value = (dynamic)Convert.ChangeType(reader.Value, valueType);

you have to be explicit with int, but all other primitive types work fine for me.

Stephen Rauch
4,31412 gold badges24 silver badges36 bronze badges
answered Oct 4, 2021 at 21:05
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.