-
Notifications
You must be signed in to change notification settings - Fork 694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor to encapsulate Recommender logic in the RecommenderPackageFeed #6082
Conversation
src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/PackageFeeds/RecommenderPackageFeed.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! This change definitely feels like it simplifies the code and makes it easier to understand :)
} | ||
|
||
private class IdentityIdEqualityComparer : IEqualityComparer<IPackageSearchMetadata> | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A pattern I prefer is to make a private constructor with a static instance to ensure that no one makes extra instances of this
{ | |
{ | |
public static IdentityIdEqualityComparer Instance = new IdentityIdEqualityComparer(); | |
private IdentityIdEqualityComparer() | |
{ | |
} |
Then line 26 can go away and line 107 can use the static singleton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot, thanks!
{ | ||
} | ||
|
||
public bool Equals(IPackageSearchMetadata x, IPackageSearchMetadata y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be OrdinalIgnoreCase.
Bug
Fixes: https://github.com/NuGet/Client.Engineering/issues/2530
Description
This refactoring focuses on improving the encapsulation and separation of concerns within the recommender logic for package feeds. The main objective is to move the logic related to the recommender feed from the generalized search object to a specialized recommender feed, thereby simplifying the code and reducing technical debt.
Key Changes:
Benefits:
This refactoring significantly improves code maintainability by isolating the recommender logic, making the codebase easier to understand and manage, and reducing the cognitive load on developers, allowing them to focus on the core responsibilities of the search object without distraction, thereby setting the stage for future enhancements and features.
PR Checklist