The first thing that leaped to my eye as that you're using List<string>
for huge strings and calling Contains
on them. This is very bad for performance, because List<>
's search is an O(n) operation - to check if an item exists, it has to go linearly through the entire collection until it's found.
The data structure you want to be using is HashSet<string>
, where checking for the existence of a given string is an O(1) operation, on average. It makes all set operations faster. Note that it can accept a StringComparer object to make it case insensitive, which saves you having to call ToLower
on every words, which also slows you down - each ToLower call creates a new String object in memory, which, for large books, will cause a lot of memory pressure.
So the first part of your method can be expressed this way:
public static void LongestPhrase(string Book1, string Book2, ref string Phrase, ref int WIndex1, ref int WIndex2)
{
string[] Words1 = Book1.Split('-');
string[] Words2 = Book2.Split('-');
// load Book1
HashSet<string> uniqueRepeatedWords
= new HashSet<string>(Words1, StringComparer.CurrentCultureIgnoreCase);
// keep only those in Book2 too.
uniqueRepeatedWords.IntersectWith(Words2);
}
Additionally, your variable naming conventions are confusing. It's customary to name local variables in lowercase (words1
, not Words1
), and it's very confusing to have variables called Words1
and Word1
, both of which being lists of words. I would name them more explicitly - allWordsInBook1
, for instance.