Skip to main content
2 of 7
added 1126 characters in body

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); 

// Find positions. 
// Notice I use a case-insensitive comparer instead of constant ToLower,
// and save the current word once to a local var instead of the 
// noisier and less clear array access every time.
// Additionally, instead of having the same line in both if and else,
// I just create the new List<int> if it doesn't exist, and 
// go back to the same incrementing code after.
var positionsInBook1 = new Dictionary<string, List<int>>(StringComparer.CurrentCultureIgnoreCase);
for (int i = 0; i < Words1.Length; i++)
{
    var word = Words1[i];
    if (uniqueRepeatedWords.Contains(word))
    {
        if (!positionsInBook1 .ContainsKey(word))
        {
            positionsInBook1.Add(word, new List<int>());
        }
        positionsInBook1[word].Add(i);
    }
}

// Now do the same find-position code for Book2 - ideally, move
// the code to different method and call it twice, with different      
// parameters.    
var positionsInBook2 = FindWordPositions(Words2, uniqueRepeatedWords);

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.