Skip to content
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

fixes for AdvancedLRU with naive implementtion, LRUCache, and AnyCallback #144

Merged
merged 7 commits into from
Nov 26, 2024

Conversation

apatrida
Copy link
Contributor

Here are some changes that fix some of the issues in these 3 tests:

Note, it should be easy to change tests to work with known solutions or the challenge solution. So one idea of that is here, basically having a "class under test" or a "function under test" although parameterized tests can be used, they would cause noise when working on the project. There are probably ways to use dynamic tests or other test providers to make this work for all solutions at the same time, without being noisy to the person solving a problem.

Also tried a few other ideas here:

  • clock for AdvancedLRU
  • generics for all collections
  • test data that is less confusing (int, int, long) are hard to remember what is what

and fixed AdvancedLRU solution problems. Note that there are problems with this question making it complex to implement both expiry and priority/lastAccess removal that were not addressed at all in the original solution. The updated solution is klunky and naive but shows the fixes required to make both work correctly at the same time. (see #142)

Another option for solutions and tests, are to have branches with the questions solved using the same tests and function/class. But that works less well when there are multiple solutions to showcase. Not sure why solutions are private (makes them hard to test and verify)

Not sure which ideas you want from here, but this shows a bit more of some of the issues present here.

@igorwojda igorwojda merged commit bc6f986 into igorwojda:main Nov 26, 2024
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.

2 participants