The "broken Window Theory" in the development

Description of broken window theory:

A House if the window is broken, no one to repair, and soon, the other windows will be inexplicably broken; a wall, if some graffiti is not washed off, soon, the wall is full of messy, unsightly things; a very clean place, people are embarrassed to throw rubbish, But once there is rubbish on the ground, people will not hesitate to throw, without any shame.

Switching to development is a situation where a lot of people think that doing a good design and everything is going to ensure the scalability and maintainability of the development process, so everyone is relaxed.

The reality is that development is a dynamic process, always meet the new requirements, our development is basically very rush, no matter what the reason, we always encounter a variety of drag progress problems. In this case, the developer unknowingly writes the code in a way that looks faster, which leads to a situation where, in the development of a particular requirement, a developer randomly opens an interface, Or simply put a certain data in a class (of course, this data can have a better placement point). So this is the beginning of a broken window theory, assuming this demand has been completed, the development of normal downward, you will find that the subsequent personnel to see this very inappropriate design principles, they began to play with a large probability of this lazy standard forward, and in the future coding process will be the original very good design dragged holes. After the last few new requirements, the maintainability of the code is reduced to the extent that it is not designed (and possibly even worse).

I did such a foolish thing during the development of the previous phase, and then I watched it go forward to the tar pits in the myth of the human Moon. Although the problem occurred in a relatively small area, a hole in the small area, a new demand, another pit. The last is the global pits.

So I am here to re-disk this broken window of the process, only hope to give their own and future developers in this regard with a certain reminder, that is to say, this broken window is too easy to happen, whether the project leader, or the project developer, we have a common responsibility to guard against this situation. Maybe we all have a good desire to do a good product, write some good code. But there are some places where negligence can lead to serious consequences that many people cannot imagine. When we break a window, we think it is just a window, and even we can hold a good wish, someone will help us mend it. It turns out that when you kick a window, more people will help you break all the windows.

In other words , the base class of the message class of the IM Transport layer is imspframe, which derives a subclass called unneedfbframe To use as a message class that does not need to be concerned with response. This unneedfbframe is only used to handle ack messages, flag messages that have been read, and so it only needs to The Sdpmessage object.

And its writing is this, that is, according to the different msgtype, the creation of the newspaper style is not, of course, this design is not good, but because the getbody method is overloaded method, so it is not annoying the future expansion.

The original class diagram is roughly like this.

In the code section below, please note that there is an add by YANGHQ below, and that's where I started digging the pits.

1  Public byte[] GetBody () {2         Switch(msgtype) {3              CaseHeartbeatack:4                 returnAssemblyimscmd.heartbeaack ();5              Casesendheartbeat:6                 returnassemblyimscmd.u_cmd_17 ();7              CaseChat_msg_ack:8String muid=string.valueof (Imsdkglobalvariable.getcurrentuid ());9                 if(muid==NULL){Ten                  return NULL; One                 } A                 returnAssemblyimscmd.ackinboxmsgrequest ( - Muid, - Messageoperator.getinboxmsgid (msg)); the              Casesend_logout: -                 returnassemblyimscmd.getlogoutrequest (); -              Casesend_mark_read_conv_msg: -                 returnassemblyconvimscmd.markreadconvmsg ( + Msg.getconversationid (), Messageoperator.getmsgid (msg)); -              CaseSend_get_max_read_conv_msgid: +                 returnAssemblyconvimscmd.getmaxreadconvmsgid (Msg.getconversationid ()); A              Casesend_get_conv_msg: at                 returnassemblyconvimscmd.getconvmsgrequest (Messageoperator.getwseq (msg), Msg.getconversationid () - , Messageoperator.getmsgid (msg), imsconfiguration.get_conv_limit); -              CaseSend_auth: -                 returnimsloginutilimpl.authrequest (); -  -             //add by YANGHQ at 20150702 increased processing of bulk get message read cursors in              CaseSend_get_conv_read_cursor_batch: -                 returnAssemblyconvimscmd.getconvreadcursorbatch (listconvid); to              Casesend_get_inbox_msg: +String uid2=string.valueof (Imsdkglobalvariable.getcurrentuid ()); -                 if(Textutils.isempty (Uid2)) { the                     return NULL; *                 } $                 returnassemblyconvimscmd.getinboxmsg (Uid2, Mlongparam, Mintparam);Panax Notoginseng             default: -                  Break; the         } +  A         return NULL; the}

In other words, after this code has been written for quite a long time, this unneedfbframe has not changed too much, that is, add some commands, these commands either do not need to add a variable, or the original sdpmessage data can be pieced together in a message.

Then to 20150702(demand should be a few days ago, the code is that day), came a demand, said is to add a signaling, is passed in a group of session ID, we want to piece together a message, sent to the server, The server will give us the last message ID of all the sessions . Due to previous design reasons, this type of message was attributed to the unneedfbframe. The problem now is that this message is not the same as the data used by the other unneedfbframe , it accepts a list, which contains the last message that needs to be taken. The session ID of the ID.

And I was obviously not a bit of object-oriented design thinking, I thought for a half day later, did not think of this list variable should be how to put it, the final decision, or put in unneedfbframe . And so the design becomes. There are two variables stacked in a class that have different meanings. And then, depending on the type of getbody , a different variable is used to generate the message at the time of the transfer.

Please note that this is the window where I kicked the door.

Because from the above code, you can see that there is a case send_get_inbox_msg below. It uses some additional data. This is because then comes a need to pick up the inbox message, the parameter is the starting message ID and the number of times to take the value of the bar.

This requirement was taken by another colleague of mine who, after seeing my writing, then kicked the window, added the variable, and added a constructor. It's done. So the class diagram becomes the following one.

I believe that this colleague of mine is a very capable colleague. Assuming there's no such thing as a kick-off window in front of me, he's probably going to be able to cope with this new demand in the right way. But because of my bad writing to later a very clear misleading: people have written like this, it is just like him. I believe that this mindset has been in the majority of developers coding process, and this mentality caused by the bad results, at least half of the first leg to kick the window of the colleague head.

After a few weeks, there is a new demand, is a message to withdraw messages, this time I have to engage in this demand, I see, not ah, I add another, is four constructors, four completely different variables placed here, and then add new requirements, I am not always add the constructor, plus switch Case.

Add up no matter, for later change code of people, must face a class in the different member values only for a specific function, while in the change, have to be particularly careful to change a value, will not affect the execution of other functions.

We say that the new message demand will certainly increase, then the N function in the same class, while the internal also has n member variables, now this is added to the fourth, I have written code is very careful to remind me that the lparam is not what I want to use, is used by a colleague in front of, do not set its value.

What if we add a long parameter later? Add a variable, or use this?? If a later maintenance colleague, see a long parameter, the direct use of it?? Does this class still work??

It was not very complicated, because of my design negligence, so that the class went to the so-called "tar Pits." I'm not going to mention the right thing here for the time being.

I'm not proposing this case to illustrate how design makes development easier. My goal is to show how a bad practice can be easily diffused and how hard it is to fix it.

The above case when I do the fourth, that is, when I recall the message, I re-recall the message as a subclass of Unneedfbframe, and implemented its GetBody method alone.

But I did not change the original to do two messages, why? Because we do not write test cases for these two functions, and this is the transport layer, those two functions have been measured several rounds, have been released. Now to change, if there is a problem, you have to count the production accident.

In other words, the two broken windows only let it break.

The correct approach should be the following, the new message demand should be as a subclass of the emergence of the call is the Imspframe.getbody method. The subclass is the key to allow the code to be open to the extension when it is responding to the demand, and for the modification to close.

The "broken Window Theory" in the development