I have a method that I feel like could be refactored more efficiently with LINQ.
The purpose of the function is to use some logic to determine which phone number to return. The logic is: Any returned number must be sms_capable. If a number was last used for an rx, use it, otherwise return the first valid number by type in this order: Other, Home, Office
string GetDefaultSMSPhoneNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
const int PHONE_TYPE_HOME = 1;
const int PHONE_TYPE_OFFICE = 3;
const int PHONE_TYPE_OTHER = 9;
var phoneNumberByType = patientNumbers.Where(p => p.sms_capable == 1).GroupBy(p => p.phone_type_id);
// Select the phone number last used in creating a prescription
if (patientNumbers.Where(p => p.sms_capable == 1 && p.last_used_for_rx == 1).Count() > 0)
{
return patientNumbers.Where(p => p.sms_capable == 1 && p.last_used_for_rx == 1).FirstOrDefault().phone_number;
}
// If no number has been used, select a configured SMS number in the following order (Other, Home, Office)
if (patientNumbers.Where(p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_OTHER).Count() > 0)
{
return patientNumbers.Where(p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_OTHER).FirstOrDefault().phone_number;
}
// If no number has been used, select a configured SMS number in the following order (Other, Home, Office)
if (patientNumbers.Where(p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_HOME).Count() > 0)
{
return patientNumbers.Where(p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_HOME).FirstOrDefault().phone_number;
}
// If no number has been used, select a configured SMS number in the following order (Other, Home, Office)
if (patientNumbers.Where(p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_OFFICE).Count() > 0)
{
return patientNumbers.Where(p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_OFFICE).FirstOrDefault().phone_number;
}
return string.Empty;
}
I know the first thing I can do is filter the list to only sms_capable numbers. I feel like I should be able to use .GroupBy to group the numbers by there type, but after they're grouped I'm not sure how to return the first non empty value? I feel like I'm looking for a way to coalesce in linq?
string GetDefaultSMSPhoneNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
const int PHONE_TYPE_HOME = 1;
const int PHONE_TYPE_OFFICE = 3;
const int PHONE_TYPE_OTHER = 9;
var phoneNumberByType = patientNumbers.Where(p => p.sms_capable == 1).GroupBy(p => p.phone_type_id);
var phoneNumber = patientNumbers.FirstOrDefault(p => p.sms_capable == 1 && p.last_used_for_rx == 1)?.phone_number;
// Doesn't work
if (string.IsNullOrEmpty(phoneNumber))
{
var number = phoneNumberByType.FirstOrDefault(p => p.Key == PHONE_TYPE_OTHER && p.Where(x => !string.IsNullOrEmpty(x.phone_number)) ||
(p.Key == PHONE_TYPE_HOME && p.Where(x => !string.IsNullOrEmpty(x.phone_number)) ||
(p.Key == PHONE_TYPE_OFFICE && p.Where(x => !string.IsNullOrEmpty(x.phone_number))));
}
I'm trying to find a concise and efficient way of filtering to the needed data. Preferably with method syntax over query syntax.
I felt like there were a lot of good answers to this question. I think though that this is perhaps the most efficient method as far as readability and number of iterations.
If I'm not mistaken this will materialize / iterate once check if there was a last used number. If so return. So minimum number of materialization / iteration is 1.
The .GroupBy on configuredNumbers should cause a materialization / iteration. After that, the OrderBy, SelectMany, and FirstOrDefault are all off the already materialized collection. There may be another iteration for the Sort / Transformation but I believe this is against the model materialized in memory for the GroupBy.
While I like the other solutions presented here, the separate predicates methods seems to me to add additional materializations / iterations of the data.
One complication to this is that the Phone Type Values can't be ordered as their values don't represent the desired priority. This can be handled with either a switch or a dictionary map I've done here with phoneTypeSortOrder. The Phone Types were moved from const's to an Enum but the values were preserved. By incorporating this translation into our GroupBy we are able to use OrderBy on our new key.
string GetDefaultSMSPhoneNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
var phoneTypeSortOrder = new Dictionary<int, int> { { (int)PhoneType.Other, 1 }, { (int)PhoneType.Home, 2 }, { (int)PhoneType.Office, 3 } };
var smsNumbers = patientNumbers.Where(p => p.sms_capable == 1);
var lastUsedNumber = smsNumbers.FirstOrDefault(p => p.last_used_for_rx == 1);
if (lastUsedNumber != null)
return lastUsedNumber.phone_number;
// If no number has been used, select a configured SMS number in the following order (Other, Home, Office)
var configuredNumbers = smsNumbers.Where(x => phoneTypeSortOrder.ContainsKey(x.phone_type_id))
.GroupBy(p => phoneTypeSortOrder[p.phone_type_id])
.OrderBy(g => g.Key)
.SelectMany(g => g)
.FirstOrDefault();
return configuredNumbers?.phone_number ?? string.Empty;
}