From 10f013415e2a391b725cd8fcec56dc593edad2c6 Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Sun, 26 Nov 2023 23:11:44 +0000 Subject: [PATCH] chore: invert `ifs`, use `TryGetValue`, remove unneeded `ToArray` --- Refit/FormValueMultimap.cs | 118 +++++++++++---------- Refit/RequestBuilderImplementation.cs | 142 ++++++++++++-------------- 2 files changed, 125 insertions(+), 135 deletions(-) diff --git a/Refit/FormValueMultimap.cs b/Refit/FormValueMultimap.cs index 86f29f435..f82114006 100644 --- a/Refit/FormValueMultimap.cs +++ b/Refit/FormValueMultimap.cs @@ -48,78 +48,76 @@ public FormValueMultimap(object source, RefitSettings settings) lock (PropertyCache) { - if (!PropertyCache.ContainsKey(type)) + if (!PropertyCache.TryGetValue(type, out var properties)) { - PropertyCache[type] = GetProperties(type); + properties = GetProperties(type); + PropertyCache[type] = properties; } - foreach (var property in PropertyCache[type]) + foreach (var property in properties) { var value = property.GetValue(source, null); - if (value != null) + if (value == null) + continue; + + var fieldName = GetFieldNameForProperty(property); + + // see if there's a query attribute + var attrib = property.GetCustomAttribute(true); + + if (value is not IEnumerable enumerable) { - var fieldName = GetFieldNameForProperty(property); + Add( + fieldName, + settings.FormUrlEncodedParameterFormatter.Format(value, attrib?.Format) + ); + continue; + } - // see if there's a query attribute - var attrib = property.GetCustomAttribute(true); + var collectionFormat = + attrib != null && attrib.IsCollectionFormatSpecified + ? attrib.CollectionFormat + : settings.CollectionFormat; - if (value is IEnumerable enumerable) - { - var collectionFormat = - attrib != null && attrib.IsCollectionFormatSpecified - ? attrib.CollectionFormat - : settings.CollectionFormat; + switch (collectionFormat) + { + case CollectionFormat.Multi: + foreach (var item in enumerable) + { + Add( + fieldName, + settings.FormUrlEncodedParameterFormatter.Format( + item, + attrib?.Format + ) + ); + } - switch (collectionFormat) + break; + case CollectionFormat.Csv: + case CollectionFormat.Ssv: + case CollectionFormat.Tsv: + case CollectionFormat.Pipes: + var delimiter = collectionFormat switch { - case CollectionFormat.Multi: - foreach (var item in enumerable) - { - Add( - fieldName, - settings.FormUrlEncodedParameterFormatter.Format( - item, - attrib?.Format - ) - ); - } - break; - case CollectionFormat.Csv: - case CollectionFormat.Ssv: - case CollectionFormat.Tsv: - case CollectionFormat.Pipes: - var delimiter = collectionFormat switch - { - CollectionFormat.Csv => ",", - CollectionFormat.Ssv => " ", - CollectionFormat.Tsv => "\t", - _ => "|" - }; - - var formattedValues = enumerable - .Cast() - .Select( - v => - settings.FormUrlEncodedParameterFormatter.Format( - v, - attrib?.Format - ) - ); - Add(fieldName, string.Join(delimiter, formattedValues)); - break; - default: - Add( - fieldName, + CollectionFormat.Csv => ",", + CollectionFormat.Ssv => " ", + CollectionFormat.Tsv => "\t", + _ => "|" + }; + + var formattedValues = enumerable + .Cast() + .Select( + v => settings.FormUrlEncodedParameterFormatter.Format( - value, + v, attrib?.Format ) - ); - break; - } - } - else - { + ); + Add(fieldName, string.Join(delimiter, formattedValues)); + break; + default: Add( fieldName, settings.FormUrlEncodedParameterFormatter.Format( @@ -127,7 +125,7 @@ public FormValueMultimap(object source, RefitSettings settings) attrib?.Format ) ); - } + break; } } } diff --git a/Refit/RequestBuilderImplementation.cs b/Refit/RequestBuilderImplementation.cs index 3ce8e3b6f..e46ba4f04 100644 --- a/Refit/RequestBuilderImplementation.cs +++ b/Refit/RequestBuilderImplementation.cs @@ -16,7 +16,7 @@ public RequestBuilderImplementation(RefitSettings? refitSettings = null) partial class RequestBuilderImplementation : IRequestBuilder { - static readonly ISet BodylessMethods = new HashSet + static readonly HashSet BodylessMethods = new HashSet { HttpMethod.Get, HttpMethod.Head @@ -74,16 +74,17 @@ Dictionary> methods { var attrs = methodInfo.GetCustomAttributes(true); var hasHttpMethod = attrs.OfType().Any(); - if (hasHttpMethod) - { - if (!methods.ContainsKey(methodInfo.Name)) - { - methods.Add(methodInfo.Name, new List()); - } + if (!hasHttpMethod) + continue; - var restinfo = new RestMethodInfoInternal(interfaceType, methodInfo, settings); - methods[methodInfo.Name].Add(restinfo); + if (!methods.TryGetValue(methodInfo.Name, out var methodInfoInternals)) + { + methodInfoInternals = new List(); + methods.Add(methodInfo.Name, methodInfoInternals); } + + var restinfo = new RestMethodInfoInternal(interfaceType, methodInfo, settings); + methodInfoInternals.Add(restinfo); } } @@ -93,64 +94,62 @@ RestMethodInfoInternal FindMatchingRestMethodInfo( Type[]? genericArgumentTypes ) { - if (interfaceHttpMethods.TryGetValue(key, out var httpMethods)) + if (!interfaceHttpMethods.TryGetValue(key, out var httpMethods)) + { + throw new ArgumentException( + "Method must be defined and have an HTTP Method attribute" + ); + } + + if (parameterTypes == null) { - if (parameterTypes == null) + if (httpMethods.Count > 1) { - if (httpMethods.Count > 1) - { - throw new ArgumentException( - $"MethodName exists more than once, '{nameof(parameterTypes)}' mut be defined" - ); - } - return CloseGenericMethodIfNeeded(httpMethods[0], genericArgumentTypes); + throw new ArgumentException( + $"MethodName exists more than once, '{nameof(parameterTypes)}' mut be defined" + ); } - var isGeneric = genericArgumentTypes?.Length > 0; + return CloseGenericMethodIfNeeded(httpMethods[0], genericArgumentTypes); + } - var possibleMethodsList = httpMethods.Where( - method => method.MethodInfo.GetParameters().Length == parameterTypes.Length - ); + var isGeneric = genericArgumentTypes?.Length > 0; - // If it's a generic method, add that filter - if (isGeneric) - possibleMethodsList = possibleMethodsList.Where( - method => - method.MethodInfo.IsGenericMethod - && method.MethodInfo.GetGenericArguments().Length - == genericArgumentTypes!.Length - ); - else // exclude generic methods - possibleMethodsList = possibleMethodsList.Where( - method => !method.MethodInfo.IsGenericMethod - ); + var possibleMethodsList = httpMethods.Where( + method => method.MethodInfo.GetParameters().Length == parameterTypes.Length + ); - var possibleMethods = possibleMethodsList.ToList(); + // If it's a generic method, add that filter + if (isGeneric) + possibleMethodsList = possibleMethodsList.Where( + method => + method.MethodInfo.IsGenericMethod + && method.MethodInfo.GetGenericArguments().Length + == genericArgumentTypes!.Length + ); + else // exclude generic methods + possibleMethodsList = possibleMethodsList.Where( + method => !method.MethodInfo.IsGenericMethod + ); - if (possibleMethods.Count == 1) - return CloseGenericMethodIfNeeded(possibleMethods[0], genericArgumentTypes); + var possibleMethods = possibleMethodsList.ToList(); - var parameterTypesArray = parameterTypes.ToArray(); - foreach (var method in possibleMethods) - { - var match = method.MethodInfo - .GetParameters() - .Select(p => p.ParameterType) - .SequenceEqual(parameterTypesArray); - if (match) - { - return CloseGenericMethodIfNeeded(method, genericArgumentTypes); - } - } + if (possibleMethods.Count == 1) + return CloseGenericMethodIfNeeded(possibleMethods[0], genericArgumentTypes); - throw new Exception("No suitable Method found..."); - } - else + foreach (var method in possibleMethods) { - throw new ArgumentException( - "Method must be defined and have an HTTP Method attribute" - ); + var match = method.MethodInfo + .GetParameters() + .Select(p => p.ParameterType) + .SequenceEqual(parameterTypes); + if (match) + { + return CloseGenericMethodIfNeeded(method, genericArgumentTypes); + } } + + throw new Exception("No suitable Method found..."); } RestMethodInfoInternal CloseGenericMethodIfNeeded( @@ -487,19 +486,12 @@ CancellationToken cancellationToken if (obj == null) continue; - if (parameterInfo != null) + //if we have a parameter info lets check it to make sure it isn't bound to the path + if (parameterInfo is { IsObjectPropertyParameter: true }) { - //if we have a parameter info lets check it to make sure it isn't bound to the path - if (parameterInfo.IsObjectPropertyParameter) + if (parameterInfo.ParameterProperties.Any(x => x.PropertyInfo == propertyInfo)) { - if ( - parameterInfo.ParameterProperties.Any( - x => x.PropertyInfo == propertyInfo - ) - ) - { - continue; - } + continue; } } @@ -511,7 +503,7 @@ CancellationToken cancellationToken // Look to see if the property has a Query attribute, and if so, format it accordingly var queryAttribute = propertyInfo.GetCustomAttribute(); - if (queryAttribute != null && queryAttribute.Format != null) + if (queryAttribute is { Format: not null }) { obj = settings.FormUrlEncodedParameterFormatter.Format( obj, @@ -657,9 +649,9 @@ bool paramsContainsCancellationToken var isParameterMappedToRequest = false; var param = paramList[i]; // if part of REST resource URL, substitute it in - if (restMethod.ParameterMap.ContainsKey(i)) + if (restMethod.ParameterMap.TryGetValue(i, out var parameterMapValue)) { - parameterInfo = restMethod.ParameterMap[i]; + parameterInfo = parameterMapValue; if (parameterInfo.IsObjectPropertyParameter) { foreach (var propertyInfo in parameterInfo.ParameterProperties) @@ -684,9 +676,9 @@ bool paramsContainsCancellationToken { string pattern; string replacement; - if (restMethod.ParameterMap[i].Type == ParameterType.RoundTripping) + if (parameterMapValue.Type == ParameterType.RoundTripping) { - pattern = $@"{{\*\*{restMethod.ParameterMap[i].Name}}}"; + pattern = $@"{{\*\*{parameterMapValue.Name}}}"; var paramValue = (string)param; replacement = string.Join( "/", @@ -706,7 +698,7 @@ bool paramsContainsCancellationToken } else { - pattern = "{" + restMethod.ParameterMap[i].Name + "}"; + pattern = "{" + parameterMapValue.Name + "}"; replacement = Uri.EscapeDataString( settings.UrlParameterFormatter.Format( param, @@ -802,9 +794,9 @@ await content } // if header, add to request headers - if (restMethod.HeaderParameterMap.ContainsKey(i)) + if (restMethod.HeaderParameterMap.TryGetValue(i, out var headerParameterValue)) { - headersToAdd[restMethod.HeaderParameterMap[i]] = param?.ToString(); + headersToAdd[headerParameterValue] = param?.ToString(); isParameterMappedToRequest = true; } @@ -1000,7 +992,7 @@ param as IDictionary } } - if (queryParamsToAdd.Any()) + if (queryParamsToAdd.Count != 0) { var pairs = queryParamsToAdd .Where(x => x.Key != null && x.Value != null)