我在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 (canDeleteSiresdeletingSearchGroup || 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
...

但是,然后您可能会被“多重返回是邪恶的”人群所羞辱。我得到的印象是发展实践就像政治...

09-30 14:06
查看更多