Skip to content

Commit

Permalink
fix some pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
KanishManuja-MS committed Jan 24, 2020
1 parent 386d0c2 commit 1795eb9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 21 deletions.
36 changes: 16 additions & 20 deletions src/Microsoft.OData.Core/Metadata/EdmLibraryExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -256,41 +256,41 @@ internal static IEnumerable<IEdmOperation> FilterBoundOperationsWithSameTypeHier
/// <summary>
/// Filters the operations by parameter count.
/// </summary>
/// <param name="functions">The operations.</param>
/// <param name="operations">The operations.</param>
/// <param name="parameterCount">The count of non-binding parameter names to match.</param>
/// <returns>The best matching operations based on parameters.</returns>
internal static IEnumerable<IEdmOperation> FindBestOverloadBasedOnParameterCount(this IEnumerable<IEdmOperation> functions, int parameterCount)
internal static IEnumerable<IEdmOperation> FindBestOverloadBasedOnParameterCount(this IEnumerable<IEdmOperation> operations, int parameterCount)
{
// The best match out of a list of candidates is the one that has the same number of (non-binding) parameters as specified.
IEnumerable<IEdmOperation> exactMatches = functions.Where(f => f.Parameters.Count() == parameterCount + (f.IsBound ? 1 : 0));
return exactMatches.Count() > 0 ? exactMatches : functions;
IEnumerable<IEdmOperation> exactMatches = operations.Where(f => f.Parameters.Count() == parameterCount + (f.IsBound ? 1 : 0));
return exactMatches.Count() > 0 ? exactMatches : operations;
}

/// <summary>
/// Filters the operations by parameter count.
/// </summary>
/// <param name="functions">The operations.</param>
/// <param name="operations">The operations.</param>
/// <param name="parameterCount">The count of non-binding parameter names to match.</param>
/// <returns>The best matching operations based on parameters.</returns>
internal static IList<IEdmOperation> FindBestOverloadBasedOnParameterCount(this IList<IEdmOperation> functions, int parameterCount)
internal static IList<IEdmOperation> FindBestOverloadBasedOnParameterCount(this IList<IEdmOperation> operations, int parameterCount)
{
IList<IEdmOperation> exactMatches = null;

// The best match out of a list of candidates is the one that has the same number of (non-binding) parameters as specified.
for (int i = 0; i < functions.Count; i++)
for (int i = 0; i < operations.Count; i++)
{
if (functions[i].Parameters.Count() == parameterCount + (functions[i].IsBound ? 1 : 0))
if (operations[i].Parameters.Count() == parameterCount + (operations[i].IsBound ? 1 : 0))
{
if (exactMatches == null)
{
exactMatches = new List<IEdmOperation>();
}

exactMatches.Add(functions[i]);
exactMatches.Add(operations[i]);
}
}

return exactMatches == null ? functions : exactMatches;
return exactMatches == null ? operations : exactMatches;
}

/// <summary>
Expand Down Expand Up @@ -320,7 +320,8 @@ internal static IList<IEdmOperation> FilterOperationCandidatesBasedOnParameterLi

bool suitableCandidate = true;

// If the number of parameters > 0 then this has to be a function as actions can't have parameters on the uri only in the payload. Filter further by parameters in this case, otherwise don't.
// If the number of binding parameters > 0 then we should only keep functions as actions can't have parameters on the uri only in the payload. Filter further by parameters in this case, otherwise don't.
// This logic might be a potential bug.
if (hasParameters)
{
if (operation.IsAction())
Expand Down Expand Up @@ -360,11 +361,6 @@ internal static IList<IEdmOperation> FilterOperationCandidatesBasedOnParameterLi
suitableCandidate = false;
}
}
else if (!operation.IsAction())
{
// If IsFunction and !IsAction are the same then this may not be required. To be confirmed.
suitableCandidate = false;
}
}
else
{
Expand Down Expand Up @@ -1935,7 +1931,7 @@ private static bool ParametersSatisfyFunction(IEdmOperation operation, IList<str
}

// if any required parameters are missing, don't consider it a match.
BitArray bitmask = new BitArray(parameterNameList.Count);
BitArray parameterExists = new BitArray(parameterNameList.Count);
foreach (var functionParameter in parametersToMatch)
{
bool matched = false;
Expand All @@ -1944,7 +1940,7 @@ private static bool ParametersSatisfyFunction(IEdmOperation operation, IList<str
if (string.Equals(parameterNameList[j], functionParameter.Name, caseInsensitive ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal))
{
matched = true;
bitmask[j] = true;
parameterExists[j] = true;
}
}

Expand All @@ -1957,7 +1953,7 @@ private static bool ParametersSatisfyFunction(IEdmOperation operation, IList<str
// if any specified parameters don't match, don't consider it a match.
for (int i = 0; i < parameterNameList.Count; i++)
{
if (!bitmask[i])
if (!parameterExists[i])
{
return false;
}
Expand Down Expand Up @@ -1994,7 +1990,7 @@ private static void EnsureOperationBoundWithBindingParameter(this IEdmOperation
throw new ODataException(ErrorStrings.EdmLibraryExtensions_UnBoundOperationsFoundFromIEdmModelFindMethodIsInvalid(operation.Name));
}

if (operation.Parameters.FirstOrDefault() == null)
if (!operation.Parameters.Any())
{
throw new ODataException(ErrorStrings.EdmLibraryExtensions_NoParameterBoundOperationsFoundFromIEdmModelFindMethodIsInvalid(operation.Name));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ internal static bool ResolveOperationFromList(string identifier, IList<string> p

// The extension method FindBoundOperations & FindOperations call IEdmModel.FindDeclaredBoundOperations which can be implemented by anyone and it could throw any type of exception
// so catching all of them and simply putting it in the inner exception.
// It is better to cast the results of ResolveBoundOperations as IList instead of looping over this for basic things such as count.
try
{
if (bindingType != null)
Expand Down

0 comments on commit 1795eb9

Please sign in to comment.