Determining the region from several sources of information

Question

The task is to determine the region by comparing three sources of information taking into account the coefficients of their importance.

//GetRegionIdByKeywordAsync returns region id by string name, if the region could not be determined, 0 will return

//The received identifiers can completely or partially coincide, or be absolutely different
int regionIdByIp = await GetRegionIdByKeywordAsync(regionByIpStr);
int regionIdByPhone = await GetRegionIdByKeywordAsync(regionByPhoneStr);
int regionIdByInput = await GetRegionIdByKeywordAsync(regionByInputStr);

var weights = new List<KeyValuePair<int, int>>
{
  //Each source of information has its own importance coefficient, determined manually
  new KeyValuePair<int, int>(regionIdByIp, 5),
  new KeyValuePair<int, int>(regionIdByPhone, 3),
  new KeyValuePair<int, int>(regionIdByInput, 4)
};

//The final decision is determined by the search for a region that has the highest score in the total
int regionId = weights.Where(x => x.Key != default(int)).GroupBy(kvp => kvp.Key).OrderByDescending(x => x.Sum(y => y.Value)).Select(x => x.Key).FirstOrDefault();

Do you see any improvement / issue?

Answer 1

Since you are not actually using a dictionary I find anonymous tuples would be much more readable here. Consider this where you don't even need a List<T> but just a simple array:

var weights = new (int RegionId, int Importance)[]
{
  (regionIdByIp, 5),
  (regionIdByPhone, 3),
  (regionIdByInput, 4)
};

Where you define such a tuple as (int RegionId, int Importance). With it the next query (also written with line breaks) would be much easier to understand.

int mostImportantRegionId = 
    weights
        .Where(x => x.RegionId > 0)
        .GroupBy(kvp => kvp.RegionId)
        .OrderByDescending(x => x.Sum(y => y.Importance))
        .Select(x => x.RegionId)
        .FirstOrDefault();

I also think that x.Key != default(int) is a strage condition. If Key is an id then it probably should be greater than 0 so it's better to write x.RegionId > 0 instead.

---

What you also can do better is the await part. Currently you wait for each task to finish one-by-one. Let's them work in parallel with WaitAll:

var regionIdByIpTask = GetRegionIdByKeywordAsync(regionByIpStr);
var regionIdByPhoneTask = GetRegionIdByKeywordAsync(regionByPhoneStr);
var regionIdByInputTask = GetRegionIdByKeywordAsync(regionByInputStr);

Task.WaitAll(new [] 
{
    regionIdByIpTask,
    regionIdByPhoneTask,
    regionIdByInputTask
};

Now you can build your weights:

var weights = new (int RegionId, int Importance)[]
{
  (regionIdByIpTask.Result, 5),
  (regionIdByPhoneTask.Result, 3),
  (regionIdByInputTask.Result, 4)
};

Answer 2

As you're currently in an async method, you should also user Task.WhenAll to wait for these three sub tasks to finish asynchronously.

var regionIdByIpTask = GetRegionIdByKeywordAsync(regionByIpStr);
var regionIdByPhoneTask = GetRegionIdByKeywordAsync(regionByPhoneStr);
var regionIdByInputTask = GetRegionIdByKeywordAsync(regionByInputStr);

await Task.WhenAll(new [] 
{
    regionIdByIpTask,
    regionIdByPhoneTask,
    regionIdByInputTask
};

Task.WhenAll Method

Creates a task that will complete when all of the supplied tasks have completed.

Source: msdn.microsoft.com

---

Edit to clarify:

Task.WaitAll will actively wait for the completion of all sub tasks and thus block the current thread.

Task.WhenAll will return Task which can be awaited. Awaiting the task will "free" the current thread to process different tasks.

Answer 3

I have just one point to add to those already made:

.OrderByDescending(x => x.Sum(y => y.Value)).Select(x => x.Key).FirstOrDefault();

A full sort is overkill when the only element required is the largest. Finding largest elements by a projection is a relatively common operation. Both are good reasons for creating a MaxBy extension method (or using MoreLINQ's) - although here you really want

MaxByOrDefault(x => x.Sum(y => y.Value), x => x.Key)