2009年3月22日星期日

If there is a dependency, Make it explicit

In "work effectively with legacy code", there is one chapter describing how to put a class, which uses a singleton object's method in its contructor, into test harness. The author argues that singleton(or global data) makes the code opacity and hard to understand. I didn't quite catch the point when I first read that. Until recently, when I tried to read a program's source code and write some documents for it, I began to understand the author's point, and found it can't be more right.

The program read some data from server, organized them and showed them in widgets. I wanted to find out what kind of data need when initializing a table. The widget class had not parameters in its constructor and not a member data which was like from server. But it had a method named "PopulateTable". It seemed to be a good place to start. I checked this method, and found it query some data through APIs in a global namespace, following it are a bunch of methods, which seemed like to process data and set up the widget. I thought that's all and continued the program. However the program stopped in a breakpoint in communication module, it really surprised me. I checked the stack, and found I miss a method named "SetXXXXX" in "PopulateTable", which took no parameter and queryed an additional data through another API in the global name space. Thank god, I had put a breakpoint in the communication module and it was the first time the program loaded the data, which would be cached in local memory and the program would had never meet the breakpoint then. Luck is the last thing we want to rely on, right? So I had to go back to the class, and searched every place where the global name space were used. And thank god again, it's just an easy case. If the class had many other classes, I had to go through all of them...

This is an example of that singleton, global API, or global data make the code hard to understand. They hide the dependency of a class in the implementation detail and make it impossible to figure out what this class needs or what this class do to the data outside. Think about an alternative design for this class, it reads the data explicitely from its PopulatesTable method, or it has a data class as parameter of its constructor, which has explicite methods to push the data into the widget class. Is it much more clear and easier to understand the code?

So what's the problem of global things? The most serious problem is that it makes developer lazy. If I have a global interface, why do I bother to add a parameter to a method or a class, not mention to think about the abstraction? As a consequence, the implementation detail lies everywhere, the code has no layers, every part of code are coupled together. Then it becomes a nightmare for the person who maintains them. Want to modify some functions(I can't understand the code in a short time)? Want to put them into a test harness( I want to have different kinds of data to test several boudary condition. But the class get the data from a global interface? How can I mock the data?)? Want to extend the function( I want to add another way to process data, but the class use some global interfaces to process data, how can I effectively reuse the original code?)? etc... Each kind of this question will kill the person.

There is another subtle implicit dependcy we don't notice, the events in GUI.
I once added a method to a plot class, the method loaded the data from file and show it. The plot had another method which would recalculate the plot data from some results. But the plot loaded from local files didn't have such kind of results, it had to query them from its parent window. So I had to add this function. "Dependency is bad", I thought. So I decided to post a event up, and the plot needed not to know who's its parent.
Did this plot depend on its parent class? It seemed not. But in fact it did. It depended on the way parent window responding to the event to guarantee it was in a good state. The very bad side of this method is it make dependency not obvious. If we don't notice this contraints and change its parent window to one that doesn't care this event, compiler can't give us any error or warning for that, then we would really be in trouble for inducing some subtle bugs.
I think about the reason for the decision using the event at that time. I find it's just a excuse that it's for avoiding the dependency. The truth is the parent window's header file are included by many other cpp files, I just didn't want to cost the compile time. Yes, it was just because I was lazy that I made that decision. It did nothing about breaking dependency. If I really wanted to break dependency, I would try to write a controller class to coordinate the data between the parent and the plot.
So think twice when you use event as a communication method between widget. In my opinion, a controller is always a better choice, it shows obvious and meaningful dependency.( If many widgets have dependency on each other, at most of time it's a sign that these widget classes break the SRP, you have to split the business logic out of them, and make the widget classes just be repsonsible for showing something)

Too many dependencies among the classes are bad, but introducing the implicit depencies do much more harm. So when there exists a dependency, make it as explicit as possible. It's not fun to surprise the person who read you code, let along to surprise the user with bug introduced by the implicit dependencies.

没有评论: