Searching for Values

Question

This question pulls content and ideas from a good many of my other questions and their answers, and pulls a lot of things together. I am quite satisfied with my approach here, but seeing as I'm refactoring my app, I might as well ask about my practices and get suggestions for improvements.

This is my search method (thank you for helping me with it, Heslacher):

private static void GetResults(ref string[] query, ref ObservableCollection<int> weight)
{
    int position = -1;

    foreach (string[] array in SearchKeys.Keys)
    {
        position++;
        int length = array.Length;
        int middle = length / 2;
        char firstCharMidArray = array[middle][0];

        foreach (string s in query)
        {
            if (s.Length == 0) continue;

            int min = array[middle][0] < s[0] ? middle : 0;
            int max = array[middle][0] <= s[0] ? array.Length : middle + 1;

            for (int i = min; i < max; i++)
            {
                weight.Add(0);

                if (array[i] == s)
                {
                    if (weight[position] == 0)
                    {
                        Data.SearchList.Add(SearchKeys.Pages[position]);
                    }
                    weight[position]++;
                }
            }
        }
    }
}

It works by pulling data from this:

public static ResourceLoader resourceFile = new ResourceLoader();

    public static string[][] Keys = { resourceFile.GetString("SearchWSOneNote").Split(' '),
                                      resourceFile.GetString("SearchWSMainMenu").Split(' '),
                                      resourceFile.GetString("SearchWSTextMenu").Split(' '),
                                      resourceFile.GetString("SearchWSTextBlockMenu").Split(' '),
                                      resourceFile.GetString("SearchWSTableMenu").Split(' '),
                                      resourceFile.GetString("SearchWSTableCellsMenu").Split(' '),
                                      resourceFile.GetString("SearchWSDrawMenu").Split(' '),
                                      resourceFile.GetString("SearchWSDrawnItemsMenu").Split(' '),
                                      resourceFile.GetString("SearchWSPictureMenu").Split(' '),
                                      resourceFile.GetString("SearchWSFileMenu").Split(' '),
                                      resourceFile.GetString("SearchWSAppBarsMenu").Split(' '),
                                      resourceFile.GetString("SearchWSCharmsMenu").Split(' '),
                                      resourceFile.GetString("SearchWSKeyboardShortcuts").Split(' '),
                                      resourceFile.GetString("SearchWSColorPalate").Split(' ')
                                      /* more here... */ };

public static MenuItem[] Pages = { new MenuItem(resourceFile.GetString("OneNote").TrimStart(' '), typeof(WindowsData.OneNote), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSMainMenu").TrimStart(' '), typeof(WindowsData.MainMenu), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSTextMenu").TrimStart(' '), typeof(WindowsData.TextMenu), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSTextBlockMenu").TrimStart(' '), typeof(WindowsData.TextBlockMenu), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSTableMenu").TrimStart(' '), typeof(WindowsData.TableMenu), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSTableCellsMenu").TrimStart(' '), typeof(WindowsData.TableCellsMenu), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSDrawMenu").TrimStart(' '), typeof(WindowsData.DrawMenu), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSDrawnItemsMenu").TrimStart(' '), typeof(WindowsData.DrawnItemsMenu), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSPictureMenu").TrimStart(' '), typeof(WindowsData.PictureMenu), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSFileMenu").TrimStart(' '), typeof(WindowsData.File), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSAppBarsMenu").TrimStart(' '), typeof(WindowsData.AppBar), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSCharmsMenu").TrimStart(' '), typeof(WindowsData.Charms), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSKeyboardShortcutsMenu").TrimStart(' '), typeof(WindowsData.KeyboardShortcuts), Menus.SearchResult),
                                   new MenuItem(resourceFile.GetString("WSColorPalateMenu").TrimStart(' '), typeof(WindowsData.ColorPalate), Menus.SearchResult) 
                                   /* more here... */ };

If you are not sure about any of the other code is called, and how it interacts with this, please feel free to ask for it or an explanation of what it does. This works because each index of the collection of search values in Keys corresponds to the index of the matching MenuItem in Pages. My keys are all stored in a .resw (resources) file for ease of adding multiple languages. I have a feeling this isn't good, although it is simple enough to maintain, so please tell me everything wrong with it.

Answer 1

Var

Use the var keyword when defining local variables where the right hand side of the definition makes the type obvious. This looks cleaner and saves time when it comes to changing types during refactoring.

e.g.

int position = -1;

should be

var position = -1;

You should also use var when declaring foreach and for loop iterators.

e.g.

for (int i = min; i < max; i++)

should be:

for (var i = min; i < max; i++)

Design

I hate seeing methods with reference variables and a void return. Unless you have a really good reason to do so, you should be returning one of those parameters. In this case you can actually do away with passing either in by reference:

• You don't appear to alter query at all, so you shouldn't pass it in using the ref keyword.
• It doesn't really make sense to pass in weight at all. Each time you call this method you'll be appending the weight data to your weight collection. If that's by design, you should state that explicitly using an AddRange or Concat method once you have the results from this function, but not by doing it in the function. This hides it as a side effect from a programmer.

So you can instead return weight normally:

private static ICollection<int> GetResults(ref string[] query)

Naming

GetResults might possibly be the only name other than Compute that could apply to any function.

Also, the name array isn't very descriptive of what that variable is. key would perhaps be closer to it. compoundKey closer still.

Answer 2

int position = -1;

foreach (string[] array in SearchKeys.Keys)
{
    position++;

This is extraordinarily suspect. In some languages, foreach or forin loops are faster than standard for loops. In others, they work out about the same. I'm pretty sure that in .NET, they're actually slower than regular for loops.

But with that said, foreach can sometimes improve readability, so they can still be nice to use. However, in this case, we're going to need a loop index anyways, so why don't we just write a regular for loop?

for (int position = 0; position < SearchKeys.Keys.Length; ++position)

---

Overall, the method could benefit greatly from some comments. You have none, and what you're doing isn't exactly simple or straight-forward. In the question, you're offering thanks to someone for helping you write it. If you required help writing it, then it's unlikely that you necessarily completely understand it. So if you later spot a bug, or you need to re-implement this in a slightly different way, are you going to understand all of this? Or if someone else is maintaining this, how much time do you want them to spend trying to figure out what the code does before they actually start improving it/fixing bugs/adding features/etc?

Search algorithms should generally be well-commented, or provide a link to some commonly used algorithm (like a wiki page or something) which you've implemented (or attempted to implement).