exceptionless / Exceptionless.Net

Exceptionless clients for the .NET platform
https://exceptionless.com
Other
557 stars 142 forks source link

Improve handling of Exception<T> #272

Closed elachlan closed 2 years ago

elachlan commented 2 years ago

Fixes #83.

Reference: https://stackoverflow.com/questions/17480990/get-name-of-generic-class-without-tilde

elachlan commented 2 years ago

How about this?

private static readonly ConcurrentDictionary<Type, string> TypeNameCache = new ConcurrentDictionary<Type, string>();
public static string GetRealTypeName(this Type t) {
    if (TypeNameCache.TryGetValue(t, out string name)) {
        return name;
    }
    if (!t.IsGenericType)
        return t.FullName.Replace('+','.');

    StringBuilder sb = new StringBuilder();
    ReadOnlySpan<char> fullName = t.FullName.AsSpan();
    int plusIndex = fullName.IndexOf('+');
    if (plusIndex > 0) {
        sb.Append(fullName.Slice(0, plusIndex).ToArray());
        sb.Append('.');
    }
    int length = fullName.IndexOf('`') - (plusIndex > 0 ? plusIndex + 1 : 0);
    sb.Append(fullName.Slice(plusIndex > 0 ? plusIndex + 1 : 0, length).ToArray());
    sb.Append('<');
    bool appendComma = false;
    foreach (Type arg in t.GetGenericArguments()) {
        if (appendComma) { sb.Append(','); }
        sb.Append(GetRealTypeName(arg));
        appendComma = true;
    }
    sb.Append('>');
    return TypeNameCache.GetOrAdd(t, sb.ToString());
}
elachlan commented 2 years ago

Should I just use the TypeNameHelper?https://github.com/exceptionless/Exceptionless.Net/blob/118b006c1c66c7fcc2c668f91af5a27cad8bd2ab/src/Exceptionless/Demystifier/TypeNameHelper.cs#L48-L53

Only issue I see is that it includes the +, so instead of: Exceptionless.Tests.Plugins.PluginTestBase.GenericException<Exceptionless.Tests.Plugins.PluginTestBase.ErrorCategory> You get: Exceptionless.Tests.Plugins.PluginTestBase+GenericException<Exceptionless.Tests.Plugins.PluginTestBase+ErrorCategory>

If this is okay, I will use that. I also suggest we add a cache of some kind to TypeNameHelper.

elachlan commented 2 years ago

@niemyjski How is that? Do you want a couple more test cases?

elachlan commented 2 years ago

@ejsmith any chance of a quick review?

elachlan commented 2 years ago

Left some comments, It's nice that we found an existing method that handles this!

All sorted. Thanks for the review!

niemyjski commented 2 years ago

Thanks for the PR! These are really nice changes @elachlan was there any other changes you wanted to get in prior to a release?

elachlan commented 2 years ago

Thanks for the PR! These are really nice changes @elachlan was there any other changes you wanted to get in prior to a release?

I won't have the time to work on this for the next two weeks.

I was going to look at using spans in place of string operations at some point.

niemyjski commented 2 years ago

I think if anything, we just report this possible optimization to the upstream dependency.

elachlan commented 2 years ago

Is this the upstream? https://github.com/benaadams/Ben.Demystifier/

niemyjski commented 2 years ago

Yes