# Determining the region from several sources of information

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?

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);

{
};


Now you can build your weights:

var weights = new (int RegionId, int Importance)[]
{
};

• No need to even Task.WaitAll - calling .Result blocks until the async action is complete. Aug 29, 2017 at 12:50
• @EBrown yes, but you don't want to wait for one task at a time but for all of them at the same time. With .Result they will run one after another and not in parallel, won't they? Aug 29, 2017 at 12:53
• Not true: if the tasks have already started then calling .Result in sequence like that effectively has the same effect as calling Task.WaitAll because they need to finish, and if you call .Result on a complete task it returns immediately. If Input finishes first, great, but that doesn't help because we still need Ip to finish. Of course, now Phone is irrelevant (because it's a lower priority) so a more clever answer might utilize the priority to effectively cancel tasks that are now irrelevant. Aug 29, 2017 at 12:55

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);

{
};


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.

• Haven't I already said that? :-] Aug 30, 2017 at 9:42
• You used Task.WaitAll which will block the current thread until all tasks are completed. Task.WhenAll will await all tasks to complete, so the Thread won't be blocked. Aug 30, 2017 at 10:55
• Oh, I see I cannot read, sorry ;-) Aug 30, 2017 at 10:56
• Yeah the calls are very similar. I've added a clarification to my awnser. Aug 30, 2017 at 11:00

.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)