Interesting idea, but.....

Jul 2, 2008 at 4:35 PM
Interesting idea, but the code isn't very good.

For example, AlphabeticalSort just doesn't work -- no matter how much we'd wish sorting would be O(N), it's not.  The algorithm would put the highest value in the last position, but otherwise wouldn't reorder the rest.  You appear to be going for a bubble sort, which (if done right) would be O(N*N).  But that's not important, since sorting (with an efficient algorithm) is built into the List<> class.   An Alphabetical sort is just:
List<String> list = .....
list.Sort();

But what about Soundex & NYSIIS sorting?  The .Net framework allows for that:
list.Sort(StringDef.SoundexOrder);
list.Sort(StringDef.NYSIISOrder);

For that to work, all we need to define is:
    public static Comparison<string> SoundexOrder
    {
        get
        {
            return delegate (string x, string y) { return Soundex(x).CompareTo(Soundex(y)); };
        }
    }
   
    public static Comparison<string> NYSIISOrder
    {
        get
        {
            return delegate (string x, string y) { return NYSIIS(x).CompareTo(NYSIIS(y)); };
        }
    }

Which brings us to the matter of Soundex().... It won't work.  A key line is:
                    if(word[i]!='a' || word[i]!='e' || word[i]!='h' || word[i]!='i'
for that expression of evaluate false, word[i] must be 'a' and' 'e' and 'h' and 'i' all at the same time.  (you want &&s between them)

Other method works, but use grossly inefficient algorithms. For example: StartsWithAlphabets.  This could be reduced to just:

public static bool StartsWithAlphabets(string word)
{
    const string valids = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
    return valids.IndexOf(word[0])!= -1;
}






Coordinator
Jul 2, 2008 at 4:50 PM
Edited Jul 2, 2008 at 4:57 PM


JamesCurran wrote:
Interesting idea, but the code isn't very good.

For example, AlphabeticalSort just doesn't work -- no matter how much we'd wish sorting would be O(N), it's not.  The algorithm would put the highest value in the last position, but otherwise wouldn't reorder the rest.  You appear to be going for a bubble sort, which (if done right) would be O(N*N).  But that's not important, since sorting (with an efficient algorithm) is built into the List<> class.   An Alphabetical sort is just:
List<String> list = .....
list.Sort();

But what about Soundex & NYSIIS sorting?  The .Net framework allows for that:
list.Sort(StringDef.SoundexOrder);
list.Sort(StringDef.NYSIISOrder);

For that to work, all we need to define is:
    public static Comparison<string> SoundexOrder
    {
        get
        {
            return delegate (string x, string y) { return Soundex(x).CompareTo(Soundex(y)); };
        }
    }
   
    public static Comparison<string> NYSIISOrder
    {
        get
        {
            return delegate (string x, string y) { return NYSIIS(x).CompareTo(NYSIIS(y)); };
        }
    }

Which brings us to the matter of Soundex().... It won't work.  A key line is:
                    if(word[i]!='a' || word[i]!='e' || word[i]!='h' || word[i]!='i'
for that expression of evaluate false, word[i] must be 'a' and' 'e' and 'h' and 'i' all at the same time.  (you want &&s between them)

Other method works, but use grossly inefficient algorithms. For example: StartsWithAlphabets.  This could be reduced to just:

public static bool StartsWithAlphabets(string word)
{
    const string valids = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
    return valids.IndexOf(word[0])!= -1;
}



Thanks a lot for pointing out the details. I will work on them now to make it better. I will also include the Comparators for Phonetic Sorting as you suggested.

Moreover it would be great, if you can point some other methods that you think
uses an inefficient algorithm and has scope for improvement.

And also if you have some idea about new methods that you would like to see in here. Please let us know that too.

Lastly would you want to join this project as a contributor?

Thanks for your time,
Sudipta Mikherjee