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 to historical figure curse filtering. #1

Merged
merged 1 commit into from
Dec 26, 2015

Conversation

Alexei-B
Copy link
Contributor

Curses (vampires, werebeasts) were being filtered by searching for exact matches to the curse name (E.G. "VAMPIRE"), however this did not match the curse strings in the active interactions. Filtering (and historical figure naming based on curses) was fixed by using a search through all active interactions for strings that contained the sub-string of the curse name.

This method for determining if a historical figure is cursed has the issue that it searches through all active interactions for the respective sub-strings, when really only an initial match is necessary. This performance hit however does not seem significant in comparison the simplicity of the solution.

Curses (vampires, werebeasts) were being filtered by searching for exact matches to the curse name (E.G. "VAMPIRE"), however this did not match the curse strings in the active interactions. Filtering (and historical figure naming based on curses) was fixed by using a search through all active interactions for strings that contained the sub-string of the curse name.

This method for determining if a historical figure is cursed has the issue that it searches through all active interactions for the respective sub-strings, when really only an initial match is necessary. This performance hit however does not seem significant in comparison the simplicity of the solution.
Kromtec added a commit that referenced this pull request Dec 26, 2015
Fixes to historical figure curse filtering.
@Kromtec Kromtec merged commit f3216ee into Kromtec:master Dec 26, 2015
@Alexei-B Alexei-B deleted the CurseFiltering branch December 26, 2015 19:12
@Kromtec
Copy link
Owner

Kromtec commented Dec 26, 2015

Awesome, thanks for contributing!

Short feedback:
Whenever you can use .Any() instead of .Count() > 0 do it.
https://dzone.com/articles/linq-performance-count-and-any

So instead of
ActiveInteractions.Where(it => it.Contains("VAMPIRE")).Count() > 0
you could use
ActiveInteractions.Count(it => it.Contains("VAMPIRE")) > 0
or even better
ActiveInteractions.Any(it => it.Contains("VAMPIRE"))

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.

None yet

2 participants