英文:
Refactor and reduce cyclomatic complexity with LINQ
问题
我觉得我的方法可以使用LINQ更高效地重构。该函数的目的是使用一些逻辑来确定要返回的电话号码。逻辑如下:任何返回的号码都必须具备短信功能。如果某个号码最后用于处方,就使用它;否则按照以下顺序返回第一个有效号码:其他(Other)、家庭(Home)、办公室(Office)。
首先,我们可以筛选只有短信功能的号码。然后,可以使用LINQ来重构代码,以更加简洁和高效的方式实现逻辑。以下是重构后的代码:
string GetDefaultSMSPhoneNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
const int PHONE_TYPE_HOME = 1;
const int PHONE_TYPE_OFFICE = 3;
const int PHONE_TYPE_OTHER = 9;
// Filter the list to only sms_capable numbers
var smsCapableNumbers = patientNumbers.Where(p => p.sms_capable == 1);
// Select the phone number last used in creating a prescription
var lastUsedForRxNumber = smsCapableNumbers.FirstOrDefault(p => p.last_used_for_rx == 1);
if (lastUsedForRxNumber != null)
{
return lastUsedForRxNumber.phone_number;
}
// If no number has been used, select a configured SMS number in the following order (Other, Home, Office)
var configuredNumber = smsCapableNumbers.FirstOrDefault(p =>
p.phone_type_id == PHONE_TYPE_OTHER ||
p.phone_type_id == PHONE_TYPE_HOME ||
p.phone_type_id == PHONE_TYPE_OFFICE);
if (configuredNumber != null)
{
return configuredNumber.phone_number;
}
return string.Empty;
}
这个重构后的代码保留了原始逻辑,但使用LINQ更加简洁和容易理解。它首先筛选出具备短信功能的号码,然后依次检查最后用于处方的号码以及按照指定顺序选择配置的号码。如果您希望使用方法语法而不是查询语法,可以根据上述代码进一步重构。
英文:
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.
答案1
得分: 0
以下是代码部分的翻译:
如果您需要按特定顺序与谓词进行匹配,可以创建一个`Func<PhoneNumbers, bool>`集合,并对其进行迭代(如果`PhoneNumbers`是一个类或记录,则不需要`Count`,如果不是,最好使用`Any`而不是count):
string GetDefaultSMSPhoneNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
const int PHONE_TYPE_HOME = 1;
const int PHONE_TYPE_OFFICE = 3;
const int PHONE_TYPE_OTHER = 9;
var predicates = new List<Func<PhoneNumbers, bool>>()
{
p => p.sms_capable == 1 && p.last_used_for_rx == 1,
p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_OTHER,
p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_HOME,
p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_OFFICE
}; // 可以移到静态字段
// 防止潜在的多次材料化源
var enumerated = patientNumbers as ICollection<PhoneNumbers> ?? patientNumbers.ToArray();
foreach (var predicate in predicates)
{
var firstOrDefault = enumerated.FirstOrDefault(predicate);
if (firstOrDefault is not null)
{
return firstOrDefault.phone_number;
}
}
return string.Empty;
}
此外,在这种特殊情况下,您可以使用.Where(p => p.sms_capable == 1)
来“预过滤”enumerated
以稍微提高性能:
// ...
var enumerated = patientNumbers
.Where(p => p.sms_capable == 1)
.ToArray();
var predicates = new List<Func<PhoneNumbers, bool>>()
{
p => p.last_used_for_rx == 1,
p => p.phone_type_id == PHONE_TYPE_OTHER,
p => p.phone_type_id == PHONE_TYPE_HOME,
p => p.phone_type_id == PHONE_TYPE_OFFICE
};
// ...
英文:
If you need matching against predicates in specific order you can create a collection of Func<PhoneNumbers, bool>
and iterate it (also if PhoneNumbers
is a class or record then you don't need Count
, if it is not, better use Any
instead of count):
string GetDefaultSMSPhoneNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
const int PHONE_TYPE_HOME = 1;
const int PHONE_TYPE_OFFICE = 3;
const int PHONE_TYPE_OTHER = 9;
var predicates = new List<Func<PhoneNumbers, bool>>()
{
p => p.sms_capable == 1 && p.last_used_for_rx == 1,
p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_OTHER,
p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_HOME,
p => p.sms_capable == 1 && p.phone_type_id == PHONE_TYPE_OFFICE
}; // Can be moved to static field
// prevent potential multiple materialization of the source
var enumerated = patientNumbers as ICollection<PhoneNumbers> ?? patientNumbers.ToArray();
foreach (var predicate in predicates)
{
var firstOrDefault = enumerated.FirstOrDefault(predicate);
if (firstOrDefault is not null)
{
return firstOrDefault.phone_number;
}
}
return string.Empty;
}
Also in this particular case you can "prefilter" the enumerated
with .Where(p => p.sms_capable == 1)
to improve performance a bit:
// ...
var enumerated = patientNumbers
.Where(p => p.sms_capable == 1)
.ToArray();
var predicates = new List<Func<PhoneNumbers, bool>>()
{
p => p.last_used_for_rx == 1,
p => p.phone_type_id == PHONE_TYPE_OTHER,
p => p.phone_type_id == PHONE_TYPE_HOME,
p => p.phone_type_id == PHONE_TYPE_OFFICE
};
// ...
答案2
得分: 0
这不使用LINQ,但可以通过将一些复杂性放入它们自己的方法中来重构代码。
private IEnumerable<IGrouping<int, PhoneNumbers>> GetSmsCapablePhoneNumbersByType(IEnumerable<PhoneNumbers> patientNumbers)
{
return patientNumbers.Where(p => p.sms_capable == 1).GroupBy(p => p.phone_type_id);
}
private PhoneNumbers GetLastUsedSmsNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
return patientNumbers.FirstOrDefault(p => p.sms_capable == 1 && p.last_used_for_rx == 1);
}
private PhoneNumbers GetFirstSmsNumberByType(IEnumerable<PhoneNumbers> patientNumbers, int phoneTypeId)
{
return patientNumbers.FirstOrDefault(p => p.sms_capable == 1 && p.phone_type_id == phoneTypeId);
}
public string GetDefaultSMSPhoneNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
var phoneNumberByType = GetSmsCapablePhoneNumbersByType(patientNumbers);
var lastUsedSmsNumber = GetLastUsedSmsNumber(patientNumbers);
if (lastUsedSmsNumber != null)
{
return lastUsedSmsNumber.phone_number;
}
var defaultSmsNumber = GetFirstSmsNumberByType(patientNumbers, PHONE_TYPE_OTHER)
?? GetFirstSmsNumberByType(patientNumbers, PHONE_TYPE_HOME)
?? GetFirstSmsNumberByType(patientNumbers, PHONE_TYPE_OFFICE);
if (defaultSmsNumber != null)
{
return defaultSmsNumber.phone_number;
}
return string.Empty;
}
如果正确执行,你的方法名称应该精确描述正在发生的事情,因此当其他人阅读你的代码时,他们应该能够通过阅读方法名称来理解发生了什么(这也意味着减少了对注释的需求)。
英文:
This isnt using linq, but you can refactor this by putting some of the complexity into their own methods
private IEnumerable<IGrouping<int, PhoneNumbers>> GetSmsCapablePhoneNumbersByType(IEnumerable<PhoneNumbers> patientNumbers)
{
return patientNumbers.Where(p => p.sms_capable == 1).GroupBy(p => p.phone_type_id);
}
private PhoneNumbers GetLastUsedSmsNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
return patientNumbers.FirstOrDefault(p => p.sms_capable == 1 && p.last_used_for_rx == 1);
}
private PhoneNumbers GetFirstSmsNumberByType(IEnumerable<PhoneNumbers> patientNumbers, int phoneTypeId)
{
return patientNumbers.FirstOrDefault(p => p.sms_capable == 1 && p.phone_type_id == phoneTypeId);
}
public string GetDefaultSMSPhoneNumber(IEnumerable<PhoneNumbers> patientNumbers)
{
var phoneNumberByType = GetSmsCapablePhoneNumbersByType(patientNumbers);
var lastUsedSmsNumber = GetLastUsedSmsNumber(patientNumbers);
if (lastUsedSmsNumber != null)
{
return lastUsedSmsNumber.phone_number;
}
var defaultSmsNumber = GetFirstSmsNumberByType(patientNumbers, PHONE_TYPE_OTHER)
?? GetFirstSmsNumberByType(patientNumbers, PHONE_TYPE_HOME)
?? GetFirstSmsNumberByType(patientNumbers, PHONE_TYPE_OFFICE);
if (defaultSmsNumber != null)
{
return defaultSmsNumber.phone_number;
}
return string.Empty;
}
If you do it correctly, your method names should describe exactly whats happening, so when somone else reads your code they should be able to follow whats happening by reading the method names (This also means there is less need for comments)
答案3
得分: 0
以下是翻译好的部分:
"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."
"如果我没记错,这将会实现/迭代一次,检查是否有最后使用的号码。如果有,就返回。所以最小的实现/迭代次数是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."
"对于configuredNumbers上的.GroupBy应该导致一次实现/迭代。之后,OrderBy、SelectMany和FirstOrDefault都是在已经实现的集合上操作的。可能还会有另一个迭代用于排序/转换,但我认为这是针对在内存中为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."
"其中一个复杂之处在于电话类型值不能按其值进行排序,因为它们的值不代表所需的优先级。这可以使用开关或字典映射来处理,我在这里使用了phoneTypeSortOrder。电话类型从const移到了Enum,但值被保留了。通过将这个翻译纳入我们的GroupBy中,我们能够在新键上使用OrderBy。"
"string GetDefaultSMSPhoneNumber(IEnumerable
"string GetDefaultSMSPhoneNumber(IEnumerable
英文:
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;
}
通过集体智慧和协作来改善编程学习和解决问题的方式。致力于成为全球开发者共同参与的知识库,让每个人都能够通过互相帮助和分享经验来进步。
评论