Generic Parse Method without Boxing
Asked Answered
C

8

14

I am trying to write a generic Parse method that converts and returns a strongly typed value from a NamedValueCollection. I tried two methods but both of these methods are going through boxing and unboxing to get the value. Does anyone know a way to avoid the boxing? If you saw this in production would you not like it, how bad is it for performance?

Usuage:

var id = Request.QueryString.Parse<int>("id");

Attempt #1:

public static T Parse<T>(this NameValueCollection col, string key)
{
    string value = col[key];

    if (string.IsNullOrEmpty(value))
        return default(T);

    if (typeof(T) == typeof(int))
    {
        //return int.Parse(value); // cannot convert int to T
        //return (T)int.Parse(value); // cannot convert int to T
        return (T)(object)int.Parse(value); // works but boxes
    }
    if (typeof(T) == typeof(long))
    {
        return (T)(object)long.Parse(value); // works but boxes
    }
    ...

    return default(T);
}

Attempt #2 (using reflection):

public static T Parse<T>(this NameValueCollection col, string key)
{
    string value = col[key];

    if (string.IsNullOrEmpty(value))
        return default(T);

    try
    {
        var parseMethod = typeof(T).GetMethod("Parse", new Type[] { typeof(string) });

        if (parseMethod == null)
            return default(T);

        // still boxing because invoke returns an object
        var parsedVal = parseMethod.Invoke(null, new object[] { value });
        return (T)parsedVal;
    }
    // No Proper Parse Method found
    catch(AmbiguousMatchException) 
    {
    }

    return default(T);
}
Croton answered 23/12, 2008 at 22:47 Comment(1)
Damien's answer here https://mcmap.net/q/826936/-primitive-type-conversion-in-generic-method-without-boxing is one simple way to achieve this. It is similar in principle to Kevin's answer here.Korry
S
8

I think you are over estimating the impact of the boxing/unboxing. The parse method will have a much bigger overhead (string parsing), dwarfing the boxing overhead. Also all the if statements will have a bigger impact. Reflection has the biggest impact of all.

I'd would not like to see this kind of code in production, as there is a cleaner way of doing it. The major problem I have with it is the large number of if statements you will need to cover all cases and the fact that someone could pass any old type to it.

What I would do is write a parse function for each type I want to parse (ie ParseInt()). It's clearer and it is well defined what the function will try to do. Also with short static methods, the compiler is more likely to inline them, saving a function call.

I think this is a bad application of generics, any particular reason for doing it this way?

Similarity answered 23/12, 2008 at 23:43 Comment(1)
Thanks Robert, the reason I posted this was because things looked fishy. I'm gonna scrap this code. I should just go with the KISS theory and not try to over architect this.Croton
D
33
public static T Parse<T>(this NameValueCollection col, string key)
{
  return (T)Convert.ChangeType(col[key], typeof(T));
}

I'm not entirely sure of ChangeType boxes or not (I guess reading the docs would tell me, but I'm pressed for time right now), but at least it gets rid of all that type-checking. The boxing overhead is not very high, though, so I wouldn't worry too much about it. If you're worried about run-time type consistency, I'd write the function as:

public static T Parse<T>(this NameValueCollection col, string key)
{
  T value;

  try
  {
    value = (T)Convert.ChangeType(col[key], typeof(T));
  }
  catch
  {
    value = default(T);
  }

  return value;
}

This way the function won't bomb if the value cannot be converted for whatever reason. That means, of course, that you'll have to check the returned value (which you'd have to do anyway since the user can edit the querystring).

Dugald answered 23/12, 2008 at 22:57 Comment(3)
Doesn't that still box? Convert.ChangeType is returning an object.Deterrence
Never said it didn't. Boxing isn't as much of a problem as people think it is.Dugald
If boxing is his concern, then the perf penalty of try-catch is much worseKorry
S
8

I think you are over estimating the impact of the boxing/unboxing. The parse method will have a much bigger overhead (string parsing), dwarfing the boxing overhead. Also all the if statements will have a bigger impact. Reflection has the biggest impact of all.

I'd would not like to see this kind of code in production, as there is a cleaner way of doing it. The major problem I have with it is the large number of if statements you will need to cover all cases and the fact that someone could pass any old type to it.

What I would do is write a parse function for each type I want to parse (ie ParseInt()). It's clearer and it is well defined what the function will try to do. Also with short static methods, the compiler is more likely to inline them, saving a function call.

I think this is a bad application of generics, any particular reason for doing it this way?

Similarity answered 23/12, 2008 at 23:43 Comment(1)
Thanks Robert, the reason I posted this was because things looked fishy. I'm gonna scrap this code. I should just go with the KISS theory and not try to over architect this.Croton
G
6

I'll add a little undocumented way:

public static T Convert<T>()
{
    if (typeof(T) == typeof(int))
    {
        int a = 5;
        T value = __refvalue(__makeref(a), T);
        return value;
    }
    else if (typeof(T) == typeof(long))
    {
        long a = 6;
        T value = __refvalue(__makeref(a), T);
        return value;
    }

    throw new NotImplementedException();
}

There is little documentation about them, but they work as of C# 4.0. Read for example here Hidden Features of C#? Remember that undocumented means unsupported, blah blah blah could not work in the future blah blah blah if you use them the devil will come for you blah blah blah :-)

Gazehound answered 7/10, 2011 at 11:51 Comment(0)
T
4

For better readability, you could use a generic dictionary with an anonymous function as follows:

var parserFuncs = new Dictionary<Type, Func<string, object>>() {
    { typeof(int), p => (int) int.Parse(p) },
    { typeof(bool), p => (bool) bool.Parse(p) },
    { typeof(long), p => (long) long.Parse(p) },
    { typeof(short), p => (short) short.Parse(p) },
    { typeof(DateTime), p => (DateTime) DateTime.Parse(p) }
    /* ...same for all the other primitive types */
};

return (T) parserFuncs[typeof(T)](value);
Truman answered 12/9, 2012 at 10:26 Comment(1)
The final T cast is where you get the boxing. Doesn't answer the question I believe. Kevin's answer is the right way to go (though more complex).Korry
M
2

Am I too late?

static Dictionary<Type, Delegate> table = 
    new Dictionary<Type, Delegate>{
        { typeof(int), (Func<string,int>)Int32.Parse },
        { typeof(double), (Func<string,double>)Double.Parse },
        // ... as many as you want
    };

static T Parse<T>(string str)
{
    if (!table.TryGet(typeof(T), out Delegate func))
        throw new ArgumentException();
    var typedFunc = (Func<string, T>)func;
    return typedFunc(str);
}

When in trouble with types, try delegates and dicts!

Mcgarry answered 25/2, 2020 at 12:54 Comment(0)
B
1

Another suggestion for implementation, using a TryParse or Parse method with a generic approach. I wrote this originally to convert strings parsed from a csv file into different types, int, decimal, list, etc.

 public static bool TryParse<T>(this string value, out T newValue, T defaultValue = default(T))
        where T : struct, IConvertible
    {
        newValue = defaultValue;
        try
        {
            newValue = (T)Convert.ChangeType(value, typeof(T));
        }
        catch
        {
            return false;
        }
        return true;
    }

    public static T Parse<T>(this string value)
        where T : struct, IConvertible
    {
        return (T) Convert.ChangeType(value, typeof (T));
    }

Here, the try parse method first sets the newValue to the default value, then tries to convert value to type T and return the newValue as type T. If the conversion fails, it returns the default value of T.

The Parse method, simply tries to do the conversion, however if its not fail safe and will throw an exception if the conversion fails.

Brandt answered 17/3, 2013 at 19:44 Comment(0)
O
0
int value = int.Parse(Request.QueryString["RecordID"]);
Osmium answered 16/11, 2009 at 17:27 Comment(0)
O
0

Here's a suggestion for implementation, following Robert Wagner's logic, but using a generic approach to reduce duplication:

public static int ParseInt32(this NameValueCollection col, string key)
{
    return Parse(col, key, int.Parse);
}
public static double ParseDouble(this NameValueCollection col, string key)
{
    return Parse(col, key, double.Parse);
}
private static T Parse<T>(NameValueCollection col, string key, Func<string, T> parse)
{
    string value = col[key];

    if (string.IsNullOrEmpty(value))
        return default(T);

    return parse(value);
}

Truth be told, returning zero for a null or empty string scares me; this could cause problems if some values are legitimately zero. Instead, I would have the methods return nullables (int?, double?, etc.), which is a slightly more compact approach than the out-parameter pattern used for the framework TryParse methods. You could do this:

public static int? ParseInt32(this NameValueCollection col, string key)
{
    return Parse(col, key, int.Parse);
}
public static double? ParseDouble(this NameValueCollection col, string key)
{
    return Parse(col, key, double.Parse);
}
private static T? Parse<T>(NameValueCollection col, string key, Func<string, T> parse)
    where T : struct    
{
    string value = col[key];

    if (string.IsNullOrEmpty(value))
        return default(T?);

    return parse(value);
}

But that would still throw an exception for non-null-or-empty strings that aren't numeric. It's better to use TryParse. The built-in Func delegates don't support ref or out parameters, so you'd have to declare your own delegate type, but that is fairly trivial.

Omland answered 3/10, 2012 at 9:2 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.