破窗理论的描述:
一个房子如果窗户破了,没有人去修补,隔不久,其它的窗户也会莫名其妙地被人打破;一面墙,如果出现一些涂鸦没有被清洗掉,很快的,墙上就布满了乱七八糟、不堪入目的东西;一个很干净的地方,人们不好意思丢垃圾,但是一旦地上有垃圾出现之后,人就会毫不犹豫地抛,丝毫不觉羞愧。
切换到开发的情况是,很多人以为做了个好的设计,就万事大吉了,就能确保开发过程中可扩展性,可维护性,这样大家都轻松了。
实际情况是,开发是个动态的过程,总是会遇到新需求的,我们的开发基本上都是很赶的,不论是什么样的原因,我们总会遇上各种拖进度的问题。这种情况下,开发人员会不知不觉以某种试乎看起来更快的方式来写代码,这样会导致一个情况,因为懒和凑合的原因,在某一次需求的开发中,某一个开发人员,随意开了一个接口,或者随便在某一个类里面放了某一个数据(当然是这个数据可以有更好的放置点)。那么这就是个破窗理论的起点,假设这次需求完成了,开发正常向下走,你会发现,后续的人员看到这种很不合适的设计原则的时候,他们就开始以一个很大的概率演着这个懒的标准前进,并在日后的编码过程中将原来挺好的设计拖的千疮百孔。最后经过几次的新需求之后,代码的维护性会降低到跟没做设计差不多的程度(甚至有可能更差)。
我在前阶段的开发过程中就干了这么一件蠢事,然后亲眼看着它向《人月神话》中的焦油坑前进。尽管问题发生在比较小的区域,但是这个小区域一个坑,再来一个新需求,又一个坑。最后就是全局填坑了。
所以我在这里复盘这个破窗的发生过程,只希望能给自已和以后的开发者在这方面以一定的提醒,那就是说,这种破窗太容易发生了, 不论是项目的主导者,或者是项目的开发者,我们都有共同的责任来警惕这种情况的发生。也许我们都有一个良好的愿望来做一个好产品,写一些好代码。但是有一些地方的疏忽可能导致的严重后果是很多人想不到的。当我们踢破一扇窗的时候,其实我们以为只是一扇窗而已,甚至我们还能持有一种美好的愿望,会有人帮我们补好它。这里的事实证明,当你踢破一扇窗的时候,会有更多人来帮你踢破所有的窗。
话说我们的IM传输层的报文类的基类是IMSPFrame,派生出一个子类叫UnNeedFbFrame来用做不太需要关注响应的报文类,最早呢这个UnNeedFbFrame只是用来处理ack消息,标记消息已读等功能,所以它只需要SDPMessage对象。
而且它的写法是这样的,也就是根据MsgType的不同,生成不生的报文体,当然这个设计谈不上好,但是由于getBody方法是重载方法,所以并不烦碍日后的扩展。
原来的类图大致是这样的。
下面代码部分,请注意,下面有一段add by yanghq的地方,就是我挖坑的起点
1 public byte[] getBody() { 2 switch(msgType) { 3 case HEARTBEATACK: 4 return AssemblyIMSCmd.heartbeaAck(); 5 case SENDHEARTBEAT: 6 return AssemblyIMSCmd.U_CMD_17(); 7 case CHAT_MSG_ACK: 8 String mUid=String.valueOf(IMSDKGlobalVariable.getCurrentUid()); 9 if(mUid==null){ 10 return null; 11 } 12 return AssemblyIMSCmd.ackInboxMsgRequest( 13 mUid, 14 MessageOperator.getInBoxMsgid(msg)); 15 case SEND_LOGOUT: 16 return AssemblyIMSCmd.getLogoutRequest(); 17 case SEND_MARK_READ_CONV_MSG: 18 return AssemblyConvIMSCmd.markreadConvMsg( 19 msg.getConversationId(), MessageOperator.getMsgid(msg)); 20 case SEND_GET_MAX_READ_CONV_MSGID: 21 return AssemblyConvIMSCmd.getMaxReadConvMsgID(msg.getConversationId()); 22 case SEND_GET_CONV_MSG: 23 return AssemblyConvIMSCmd.getConvMsgRequest(MessageOperator.getWseq(msg), msg.getConversationId() 24 , MessageOperator.getMsgid(msg), IMSConfiguration.GET_CONV_LIMIT); 25 case SEND_AUTH: 26 return IMSLoginUtilImpl.authRequest(); 27 28 // add by yanghq at 20150702 增加对批量获取消息已读游标的处理 29 case SEND_GET_CONV_READ_CURSOR_BATCH: 30 return AssemblyConvIMSCmd.getConvReadCursorBatch(listConvID); 31 case SEND_GET_INBOX_MSG: 32 String uid2=String.valueOf(IMSDKGlobalVariable.getCurrentUid()); 33 if(TextUtils.isEmpty(uid2)){ 34 return null; 35 } 36 return AssemblyConvIMSCmd.getInboxMsg(uid2, mLongParam, mIntParam); 37 default: 38 break; 39 } 40 41 return null; 42 }
话说,在这段代码写完以后相当长的时间里,这个UnNeedFbFrame没有发生过太大的变化,也就是加了一些命令,这些命令要么不需要新增变量,要么就是用原来的SDPMessage中的数据就能拼凑出报文。
再到20150702(需求应该是前几天来的,编码是那天),来了一个需求,说是要增加一种信令,是传进来一组的会话ID,我们要拼凑出报文,发送给服务端,服务端会给我们下发所有会话的最后一条消息ID。由于之前的设计原因,这个类型的报文被归到了UnNeedFbFrame。现在的问题是,这个报文比之前其它的UnNeedFbFrame用到的数据是不一样的,它接受的是一个list,里面放着需要获取最后一条消息ID的会话ID。
而我当时明显没有半点面向对象的设计思维,我在想了半天以后,没想出这个list变量应该要怎么放呢,最后决定,还是放在UnNeedFbFrame中去。于是乎,设计就变成了这样。一个类中堆放着两个意义不同的变量。然后再根据传进来type不同,在getBody的时候,用不同的变量来生成报文。
请注意,这就是我踢破的那扇窗户。
因为从上面的代码当中,你可以看到下面还有一个case SEND_GET_INBOX_MSG。它用到了另外的一些数据。这是因为之后又来了一个需求,是需要去取收件箱消息的,需要参数是起始的消息iD和一次取多少条的值。
这个需求是接手我的另一个同事做的,他看到我的写法之后,接着踢窗户,再把变量加上去,再加一个构造函数。搞定,收工。于是类图变成下面这张了。
我相信我的这个同事是一个能力不错的同事。假设没有我前面这种踢破窗户的写法,他很有可能是会用正确的方法来应对这种新增的需求的。但是由于我的烂写法给后来一种很明确的误导:人家都这样写了,那就跟他一样就好了嘛。我相信这种心态在大部分的开发人员编码过程中都曾经出现过了,而这种心态导致的不好结果,至少有一半要算在那个第一脚踢破窗户的同事头上。
过了几周,又有一个新需求来了,是一个撤回消息的报文,这次又换我来搞这个需求了,我一看,不对啊,我再加一个,就是四个构造函数了,四个完全不同意义的变量放在这里面,以后再加新需求,我是不是一直加构造函数,加Switch Case。
加完没有关系,对于以后改代码的人来说,必然面对一个类中不同成员值只用于特定功能,同时在改动的时候,不得不特别小心改一个值,会不会影响到别的函数的执行。
我们说新报文需求肯定是会增加的,那么把N个功能放在同一个类里,同时内部还有N个成员变量,现在这是增加到第四个,我已经写代码的时候很小心的提醒自已,那个lParam不是我要用的,是前面一个同事用的,千万不要去设它的值。
如果以后再加一个long参数呢?再加一个变量,还是用这个??如果某一个后来维护的同事,看到有一个long参数,直接就用了呢??这个类还能正常工作吗??
本来不是很复杂的事情,因为我设计上的疏忽,让这个类走向了所谓的“焦油坑”。我在这里暂时不去提正确的做法是什么。
我把这个案例提出来不是为了说明设计如何让开发更轻松。我的目的在于演示一个坏的做法是如何很轻松的扩散出去的,而且修复它是多么艰难的一件事情。
上面的案例当我做到第四个的时候,也就是撤回消息的时候,我重新把撤回消息的报文作为UnNeedFbFrame的子类,单独实现了它的getBody方法。
但是我没有去改动原来做做的两个报文,为什么?因为我们没有针对这两个功能写测试用例,而且这又是传输层,那两个功能已经是提测过几轮,都已经发布了。现在去改,要是出了问题,就得算生产事故了。
也就是说实际上那两个破掉的窗户只能让它破了。
而正确的做法应该是下面这样的,新的报文需求应该是作为子类的出现的,调用处都是调IMSPFrame.getBody方法。而子类才是让编码在应对需求的时候能够实现对扩展开放,对修改关闭的关键。