代码改变世界

代码重构之 —— 一堆if、esle 逻辑的处理

2014-05-21 17:01  麦舒  阅读(12955)  评论(98)  编辑  收藏  举报

这几天，接手一个同事的代码，关于微信接口开发的，那一堆的 if，看得哥蛋痛了，这个毛病也是很多新手容易犯的，所以特地把这次重构写出来。

下面来我们看看这个代码的问题所在，if else 里面的代码块逻辑，不好改，使得它的重用性为 0，并且难以阅读。当然，如果 if 只有一两个，或者3个，这样写是问题不大的。

但是如果多了，这种代码便会让维护变得困难起来。

if (strMsgType == "text")
{
    textContentClient = rootElement.SelectSingleNode("Content").InnerText;
    strResult = SetMsgType_Text(strClientName, textContentClient, db, strServerName, Identity);
    System.Diagnostics.Trace.WriteLine(strResult);

    return Content(strResult);
}
else if (strMsgType == "event")
{
    string eventType = rootElement.SelectSingleNode("Event").InnerText.ToLower();
    if (eventType == "subscribe")
    {
        string keyCode = "";
        if (rootElement.SelectSingleNode("EventKey") != null)
            keyCode = rootElement.SelectSingleNode("EventKey").InnerText.ToLower();

        strResult = FormatEventSubscribe(keyCode);


        RecordReplyMessage();

        return Content(strResult);
    }
    else if (eventType == "scan")
    {
        string keyCode = rootElement.SelectSingleNode("EventKey").InnerText.ToLower();

        var outLetName = "欢迎关注";
        var outletDB = ShoppingContext.CreateInstance(Identity);
        var outLetModel = outletDB.Outlets.FirstOrDefault(o => o.SceneId == Int32.Parse(keyCode));
        if (outLetModel != null)
            outLetName += outLetModel.Name;


        return Content(GetTextTemp(strClientName, strServerName, outLetName));
    }
    else if (eventType == "click")
    {
        string keyCode = rootElement.SelectSingleNode("EventKey").InnerText.ToLower();
        strResult = FomatMenuMessage(keyCode);
        return Content(strResult);

    }
    else if (eventType == "unsubscribe")
    {
        var subIds = db.ReplyRecords.Where(r => r.FromOpenId == this.ClientId.ToString() && r.EMessType == EEventType.Subscribe.ToString() && r.KeyWord != null).Select(o => o.KeyWord).ToArray();
        var unSubIds = db.ReplyRecords.Where(r => r.FromOpenId == this.ClientId.ToString() && r.EMessType == EEventType.Unsubscribe.ToString() && r.KeyWord != null).Select(o => o.KeyWord).ToArray();

        var SencesId = "";
        foreach (var k in subIds)
        {
            if (!unSubIds.Contains(k))
            {
                this.ReplyModel.KeyWord = k;
                break;
            }
        }

        this.ReplyModel.EMessType = EEventType.Unsubscribe.ToString();
        RecordReplyMessage();
    }
}
else if (strMsgType.ToLower() == "location")
{
    string strLocation_X = rootElement.SelectSingleNode("Location_X").InnerText;
    string strLocation_Y = rootElement.SelectSingleNode("Location_Y").InnerText;

    strResult = FormatOutLetLBS(double.Parse(strLocation_X), double.Parse(strLocation_Y), 10);
    //strResult = FormatTextMessage(strLocation_X + "|" + strLocation_Y);
    return Content(strResult);
}
else if (strMsgType.ToLower() == "image")
{
    string strImgUrl = rootElement.SelectSingleNode("PicUrl").InnerText;
}

一种比较好的处理方法就是把语句块内的代码抽出来，写成函数，如下面所示：

    public class MessageProcesser
    {
        public ReplyMessage Process(string xml)
        {
            var msg = PostMessage.FromXml(xml);
            switch (msg.MsgType)
            {
                case Models.PostMessageType.Event:
                    var eventType = ((EventMessage)msg).Event;
                    switch (eventType)
                    {
                        case EventType.Click:
                            return ProcessClickEvent((ClickEvent)msg);
                        case EventType.Location:
                            return ProcessLocationEvent((LocationEvent)msg);
                        case EventType.Scan:
                            return ProcessScanEvent((ScanEvent)msg);
                        case EventType.Subscribe:
                            return ProcessSubscribeEvent((SubscribeEvent)msg);
                        case EventType.Unsubscribe:
                            return ProcessUnsubscribeEvent((UnsubscribeEvent)msg);
                    }
                    break;
                case Models.PostMessageType.Image:
                    return ProcessImageMessage((ImageMessage)msg);
                case Models.PostMessageType.Link:
                    return ProcessLinkMessage((LinkMessage)msg);
                case Models.PostMessageType.Location:
                    return ProcessLocationMessage((LocationMessage)msg);
                case Models.PostMessageType.Text:
                    return ProcessTextMessage((TextMessage)msg);
                case Models.PostMessageType.Video:
                    return ProcessVideoMessage((VideoMessage)msg);
                case Models.PostMessageType.Voice:
                    return ProcessVoiceMessage((VoiceMessage)msg);
            }
            return null;
        }

        protected virtual ReplyMessage ProcessClickEvent(ClickEvent msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessLocationEvent(LocationEvent msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessScanEvent(ScanEvent msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessSubscribeEvent(SubscribeEvent msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessUnsubscribeEvent(UnsubscribeEvent msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessImageMessage(ImageMessage msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessLinkMessage(LinkMessage msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessLocationMessage(LocationMessage msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessTextMessage(TextMessage msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessVideoMessage(VideoMessage msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage ProcessVoiceMessage(VoiceMessage msg)
        {
            return DefaultProcess(msg);
        }

        protected virtual ReplyMessage DefaultProcess(PostMessage msg)
        {
            var reply = new TextReply(msg);
            if (msg.MsgType == PostMessageType.Event)
            {
                reply.Content = string.Format("{0} event is not processed.", ((EventMessage)msg).Event);
            }
            else
            {
                reply.Content = string.Format("{0} message is not processed.", msg.MsgType);
            }
            return reply;
        }


    }

在使用的时候，继承上面的类就行了。

public class MyMessageProcesser : WeiXin.MessageProcesser
{
    public MyMessageProcesser()
    {
    }

    protected override ReplyMessage ProcessSubscribeEvent(SubscribeEvent msg)
    {
        var reply = new TextReply(msg);
        reply.Content = "你好，欢迎关注";
        return reply;
    }

    protected override ReplyMessage ProcessUnsubscribeEvent(UnsubscribeEvent msg)
    {
        var reply = new TextReply(msg);
        reply.Content = "取消关注";
        return reply;
    }
}

欢迎讨论，欢迎板砖。

=======================================================

有朋友说，我只是把 if 换成了 switch，这个没错，但更重要的是，换成了一个可重写方法的类。

 

有朋友说，使用命令模式会不会更好点。是这样的，因为是微信的接口，以后就算是增加，也是很少会发生的，并且要作的改动也不多。所以不想变得太复杂了。现在是 if 内的语句块需要变动，因为要面对不同的用户，他们的处理都是不同的。

 

有不少朋友都误会了，以为我是为了消除 if，else，在这里，if、else 带来的问题只是阅读读上的不便，真正要害的的地方是if、else间的逻辑代码块，这些代码会因为不同的客户，做出不同的处理，每换一个客户，都泛及到修改里面的代码，所以很有必要对它进行重构。（很多朋友都抓我 switch case 的小辨子，因为特地强调一下这里）

为什么不用命令模式、或者把类型与处理方法保存在键值对。我们先来考虑一个问题，这些类型有没有增加的可能性？有，但是这个慨率比较小的，就算发生了，修改的正本也是非常低的，而且，当然你增加了一个后，以后需要增加一个的慨率更加小了。这些类型的数量，不会大大地增加的。

总结一下这种处理的好处：对于开发人员来说，它非常便于阅读和理解，而对于使用者来说，通过重载来实现，也是很容易接受的。

 

认同者寡，而反对者众，我是不是可以这么认为，能把代码写好的人，还是少数的。

你们如果觉得我太懂得自我安慰了，请给我一板砖，把我给拍醒吧。^_^

 

看这位朋友这么热心给我留言，我就花点时间来回答一下这个问题： 

========================================================

提出以下疑问
首先:
var msg = PostMessage.FromXml(xml);
这个方法其实已经将msg实例成一个具体的类型了
下面的判断为什么不是针对
msg is Type 而是判断字符串或枚举?
后面的全是强转,不怕报错吗?

然后:
既然第一句就已经区分了msg的类型
为什么不可以直接
var msg = PostMessage.FromXml(xml);
msg.Process();

还需要如此多的判断?

----------------------------------------------------------------------------

回答：

首先，你得了解我的背景，也就是这个代码所应用的情景，面对不同的客户，处理的逻辑都是不同的。

var msg = PostMessage.FromXml(xml);
msg.Process(); 

能够做到，不同的客户，不同的实现吗？

如果强转换会出错，那肯定是开发人员的错。^_^  难道你写完代码了，不检查，也不测试的吗？

================================================

疑问：

而改用基类
protected virtual ReplyMessage ProcessScanEvent(Message msg)

这样可以把判断类型(或者说转换类型)的工作交给具体实现去完成
可以避免在公共方法中的越权控制(你不能确定实现类是否可以处理一种新类型)
----------------------------------------------------------------------------
回答：
我希望调用者容易使用。如果把这个工作扔给调用者，调用的次数多了，这个量就会变得大了。造成调用者容易出错。
================================================
 
附录一篇另外一位作者对于此文的吐槽：
http://www.cnblogs.com/guaiguai/p/3752415.html
 

千呼万唤岂出来，写款软件不容易——Visual Entity 2.0 发布

================================================
 
 

大家如果对我做的东西感兴趣，可以和我联系：

QQ： 81932759

Q群一： 71418067

Q群二： 88718955

上海的朋友，可以扫一扫下这面这个公众号，建这个公众号的目的，希望能够通过组织一些线下活动，让更多的程序员相互认识。