HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Analysing the results of various search engines and determining a winner

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thedeterminingsearchengineswinnerandresultsanalysingvarious

Problem

This is a programming challenge I submitted as part of a job interview, which I failed because it lacked "maintainability" and "patterns and best industry practices", so I guess we could all learn something from my mistakes.

The challenge was to write a program that, given a list of programming languages, it'd return a list of result counts in various search engines, a winner on each engine, and a total winner:

C:\> searchfight.exe .net java
.net: Google: 4450000000 MSN Search: 12354420
java: Google: 966000000 MSN Search: 94381485
Google winner: .net
MSN Search winner: java
Total winner: .net


I was not allowed to use any external libraries, so no HtmlAgilityPack or stuff like that. I will only post what I consider the most relevant sections of the code to keep it short, but I also just uploaded the whole project to GitHub.

So here is my Main function:

static void Main(string[] args)
{
    try
    {
        Run(args);
    }
    catch (Exception ex)
    {
        Console.WriteLine();
        Console.WriteLine("An unexpected exception has occurred: " + Environment.NewLine + ex.ToString());
    }
}


Here is Run:

```
private static void Run(string[] args)
{
try
{
if (args.Length == 0)
throw new ConfigurationException("Expected at least one argument.");

var runners = ReadConfiguration().SearchRunners.Where(runner => !runner.Disabled).ToList();
var results = CollectResults(args, runners).Result;

Console.WriteLine();

ConsoleHelpers.PrintAsTable(results.Languages, results.Runners, results.Counts, "{0:n0}"); // Using 'ConsoleHelpers.PrintAsList' will print as a list instead.

Console.WriteLine();

ConsoleHelpers.PrintAsTable(
new[] { "Winner" },
results.Winners.Select(winner => winner.Key).ToList(),
new[] { results.Winners.Select(w => w.Value).ToList() }.ToRectangularArray()
);

Console.WriteLine();

Console.Wri

Solution

Your code is really hard to follow. I think it is beacause the API you used is just way too complex for such a straightforward task. It's also weird. Like, why would result class... collect itself? I had troubles wrapping my head around this stuff.

I think you should keep it simple:

interface ISearchEngineFactory
{
    ISearchEngine[] CreateEngines();
}

interface ISearchEngine
{
    string Name { get; }
    Response Send(string query);
}

public class Response 
{
    public int HitCount { get; set; }
    public string Query { get; set; }
    public string SourceName { get; set; }
}

public class Result
{
    //make sure to synchronize this method
    public void Aggregate(Response response)
    {
        ...
    }

    //override it to return required output
    public override string ToString()
    {
        ...
    }
}


Your Main function will then look like this (you can wrap it in try catch if you feel like it):

//move your initialization logic to factory
ISearchEngineFactory factory = new ConfigurationFactory();
var engines = factory.CreateEngines();
var result = new Result();
Parallel.ForEach(engines, engine => 
                                {
                                    foreach(var query in args)
                                    {
                                        var response = engine.Send(query);
                                        result.Aggregate(response);
                                    }
                                });
Console.WriteLine(result);


Some things can still be improved. You could add a timeout parameter to Send method, for example. But hopefully you get the idea.

As for your WebClient implementation - I can't really comment on that, since I am not experienced enough in web development. Maybe someone else will. :)

Code Snippets

interface ISearchEngineFactory
{
    ISearchEngine[] CreateEngines();
}

interface ISearchEngine
{
    string Name { get; }
    Response Send(string query);
}

public class Response 
{
    public int HitCount { get; set; }
    public string Query { get; set; }
    public string SourceName { get; set; }
}

public class Result
{
    //make sure to synchronize this method
    public void Aggregate(Response response)
    {
        ...
    }

    //override it to return required output
    public override string ToString()
    {
        ...
    }
}
//move your initialization logic to factory
ISearchEngineFactory factory = new ConfigurationFactory();
var engines = factory.CreateEngines();
var result = new Result();
Parallel.ForEach(engines, engine => 
                                {
                                    foreach(var query in args)
                                    {
                                        var response = engine.Send(query);
                                        result.Aggregate(response);
                                    }
                                });
Console.WriteLine(result);

Context

StackExchange Code Review Q#111172, answer score: 5

Revisions (0)

No revisions yet.