Searching a dictionary

Question

After some advice, which method do you think is better to use when searching a dictionary for the matching value(s)?

Method One

Console.WriteLine("ENTER A PERSON/NUMBER TO SEARCH FOR");
string input = Console.ReadLine().ToUpper();
bool found = false;

if (myPhoneBook.ContainsKey(input)) // used with a persons name and returns number
{
    Console.WriteLine(myPhoneBook[input]);
    found = true;
}
if (!(myPhoneBook.ContainsKey(input))) //searches the number and returns the name
{
    var searchingfornumbers = from num in myPhoneBook
                              where num.Value == input
                              select num.Key;

    foreach(var person in searchingfornumbers)
    {
        Console.WriteLine(person);
        found = true;
    }
}
if(found == false)
{
    Console.WriteLine("NO RECORD FOUND");
}

Method Two

Console.WriteLine("ENTER A PERSON/NUMBER TO SEARCH FOR");
string input = Console.ReadLine();
bool found = false;
var query0 = from person in myPhonebook //searches for the number, returns name
             where person.Value == input
             select person.Key;

foreach(var person in query0)
{
    Console.WriteLine(person);
    found = true;
}

var query1 = from person in myPhonebook //searches for the name, returns numbers
             where person.Key == input
             select person.Value;

foreach (var person in query1)
{
    Console.WriteLine(person);
    found = true;
}
if(found == false)
{
    Console.WriteLine("NO RECORD FOUND");
}
Console.ReadKey();

Answer 1

I don't like either of them because they look like multiple numbers could be assigned to multiple persons which is obviously wrong.

You can get only one result for a search because you are not searching with wildcards so consider this. A person can have multiple numbers so for a person you can have a single hit. If you search for an exact number you'll also get a single hit. This can be made clear by naming the method appropriately: FindExact. Later you can add another one that can search with wildcards (patterns).

class Phonebook
{
    private readonly Dictionary<string, List<string>> _entries = 
        new Dictionary<string, List<string>>();

    public void Add(string person, string number)
    {
        List<string> numbers;
        if (_entries.TryGetValue(person, out numbers))
        {
            numbers.Add(number);
        }
        else
        {
            _entries[person] = new List<string> { number };
        }
    }

    public KeyValuePair<string, List<string>> FindExact(string personOrNumber)
    {
        List<string> numbers;
        return
            _entries.TryGetValue(personOrNumber, out numbers)
            ? new KeyValuePair<string, List<string>>(personOrNumber, numbers)
            : _entries.FirstOrDefault(x => x.Value.Contains(personOrNumber));
    }
}

Currently searching for a persons would be faster because it is a key in the dictionary. You could add another dictionary that maps numbers to persons and make the number search as fast as person search by looking at the key instead of searching all values:

private readonly Dictionary<NUMBER, PERSON> _numberPerson...

---

In your second solution you don't need a dictionary at all because use don't use it like it should be used. You loop twice over it to find either value. In this case you can also use a list.

If you want to use a dictionary then use it appropriately otherwise it doesn't make any sense.

Use two dictionaries to make the exact search even faster but don't loop twice over it.

You can also replace both queries with a single one:

var query = 
    from x in myPhonebook
    where x.Key == input || x.Value == input
    select x;

Even better like so:

var result = myPhonebook.FirstOrDefault(x => x.Key == input || x.Value == input);

Answer 2

I would prefer the second method. It prints out more possibilities, making it more likely that what the user is looking for is being printed out. Further down I make a point about adding the results to a list instead of printing them out immediately. Using such logic would allow you to sort the list so that relevant results could be displayed first with less likely, but possible matches further down.

There are also some style points I would like to make:

1. When using a boolean variable in an if statement you never need to do if(booleanVariable == false) you can simply do if(!booleanVariable)
2. Its not entirely clear that checking if the dictionary has the available key means that you are searching by name (as opposed to number). I would extract that into a method called searchByName() and then use if(searchByName()). This is a lot more human readable and can allow you to get rid of the comment explaining what the code does. (In general if you feel like you need a comment to say what the code is doing, the code could use a bit of change to make it more self explanatory.)

3. Instead of using multiple but separate if's, use if-else.

   if (searchByName()) { //search by name functionality } else { //search by number functionality }

Alternatively:

if (searchByName()) {
    //search by name functionality
} else if(searchByNumber()) {
    //search by number functionality
}

This makes it more clear what it being checked for in each if statement and what is expected to happen inside that block of code.

4. Don't use flags such as found. A much better option would be to add all the results into a List instead of printing them out right then and there. You can then print what is in the list if it has results or print that no results were found if the list is empty. As mentioned earlier this also allows you to present the results in a more meaningful way to the user either by sorting by relevancy or allowing them to filter out so they only see names or only see numbers etc.