Skip to content

Conversation

@MementoMorj
Copy link
Contributor

dev

JIRA

Code reviewers

  • @github_username

Second Level Review

  • @github_username

Summary of issue

ToDo

Summary of change

ToDo

Testing approach

ToDo

CHECK LIST

  • СI passed
  • Сode coverage >=95%
  • PR is reviewed manually again (to make sure you have 100% ready code)
  • All reviewers agreed to merge the PR
  • I've checked new feature as logged in and logged out user if needed
  • PR meets all conventions

Comment on lines +60 to +67
public void RemoveStreetcodeCaches(int streetcodeId)
{
string cacheKeyImage = $"ImageCache_{streetcodeId}";
string cacheKeyText = $"TextCache_{streetcodeId}";

_cache.Remove(cacheKeyImage);
_cache.Remove(cacheKeyText);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's implement here the generic Remove method, this method shouldn't know anything about the method where it will be used.

}

return Result.Ok(imageDtos);
string cacheKey = $"ImageCache_{request.StreetcodeId}";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this cache keys to some standardized solution, we shouldn't hardcode this key here and in every method we need this

public async Task<Result<TextDTO?>> Handle(GetTextByStreetcodeIdQuery request, CancellationToken cancellationToken)
{
var text = await _repositoryWrapper.TextRepository
string cacheKey = $"TextCache_{request.StreetcodeId}";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same code smell here

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 22 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@MementoMorj MementoMorj merged commit 1c1189e into develop Sep 13, 2023
@Lazy-Lenny Lazy-Lenny deleted the cashing-for-andrii branch March 28, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants