我在C#中有此方法,并且希望对其进行重构。 bool 值和行过多。什么是最好的重构。上一堂新课似乎有些过头了,而简单地一分为二似乎很难。任何见解或指针将不胜感激。
重构方法
private DialogResult CheckForSireRestrictionInSubGroup(bool deletingGroup,string currentId)
{
DialogResult result = DialogResult.No;
if (!searchAllSireList)
{
DataAccessDialog dlg = BeginWaitMessage();
bool isClose = false;
try
{
ArrayList deletedSire = new ArrayList();
ISireGroupBE sireGroupBE = sireController.FindSireGroupSearch();
if (sireGroupBE != null)
{
//if the current group is in fact the seach group before saving
bool currentGroupIsSeachGroup = sireGroupBE.TheSireGroup.id == currentId;
//if we have setting this group as search group
bool selectedAsSearchGroup = this.chkBoxSelectedSireGroup.Checked;
//if the group we currently are in is not longer the seach group(chk box was unchecked)
bool wasSearchGroup = currentGroupIsSeachGroup && !selectedAsSearchGroup;
//if the group is becoming the search group
bool becomesSearchGroup = !currentGroupIsSeachGroup && selectedAsSearchGroup;
//if the group being deleted is in fact the search group
bool deletingSearchGroup = deletingGroup && currentGroupIsSeachGroup;
//if the user checked the checkbox but he's deleting it, not a so common case, but
//we shouldn't even consider to delete sire in this case
bool deletingTemporarySearchGroup = deletingGroup && !currentGroupIsSeachGroup;
//if we are not deleting a temporary search group and it's either
//becoming one (without deleting it) or we already are the search group
bool canDeleteSires = !deletingTemporarySearchGroup &&
(becomesSearchGroup || currentGroupIsSeachGroup);
//we only delete sires if we are in search group
if (canDeleteSires)
{
if (deletingSearchGroup || wasSearchGroup)
{
// If we deleted all sires
deletedSire = new ArrayList();
deletedSire.AddRange( sireGroupBE.SireList);
}
else
{
//if we delete a few sire from the change of search group
deletedSire = GetDeleteSire(sireGroupBE.SireList);
}
}
EndWaitMessage(dlg);
isClose = true;
result = ShowSubGroupAffected(deletedSire);
}
}
finally
{
if (!isClose)
{
EndWaitMessage(dlg);
}
}
}
return result;
}
最佳答案
一种选择是将每个主 bool (canDeleteSires
,deletingSearchGroup || wasSearchGroup
)重构为具有描述逻辑的可读版本的名称的方法:
if (WeAreInSearchGroup())
{
if (WeAreDeletingAllSires())
{
deletedSire = new ArrayList();
deletedSire.AddRange( sireGroupBE.SireList);
}
else
{
deletedSire = GetDeleteSire(sireGroupBE.SireList);
}
}
然后,您将当前的 bool 逻辑封装在这些方法中,如何传递状态(方法参数或类成员)只是一个问题。
这会将 bool 值从main方法中删除为直接询问和回答问题的较小方法。我已经看到在“评论是邪恶的”开发风格中使用了这种方法。老实说,如果您是独狼,我会觉得这有点过分,但是在团队中,阅读起来会容易得多。
出于个人喜好,我还将反转您的第一个if语句以尽早返回,这将降低整个方法的缩进级别:
if (searchAllSireList)
{
return result;
}
DataAccessDialog dlg = BeginWaitMessage();
bool isClose = false;
try
...
但是,然后您可能会被“多重返回是邪恶的”人群所羞辱。我得到的印象是发展实践就像政治...