Refactoring "Badly Written" C Source Code for Embedded System

Ok. I'm not an expert on this "Refactoring" subject. I'm tired, sleepless but have to write this because it's very important. I'm gonna lost it on the way to bed if I don't write it now.

So, you have this C code for embedded system written by someone at some hardware company, let's call it X. It's written like someone rushing out to get the next train, and then some other guy come, add some code and some #define here and there. You're the "lucky" guy at another company, let's call the company Y. Y have bought large amount of hardware from X to customize for their specific market. Now, your manager at company Y want you to strip off all unnecessary code from the code that company X releases. In addition, your manager at your company also want you to add some "small" feature not yet exist in the C code produced by X for the specific hardware that your company bought. However, that feature exist in other product from company X which still in the same "family" as the product Y have bought. You found out that the "small" feature is controlled by conditional compilation flag, but with a twist. The flag enables the feature that you need along with some other feature that you don't need. What would you do when the C code cluttered with conditional compilation construct almost everywhere? What would you do to refactor the code so that it can become manageable and you can sleep well at night after that?

Let's see. This is what I would do:

• Take a lesson (or two) from Busybox coding style. Replace long #ifdef with
  function call. The new function enclosed in the same #ifdef and called from previous location and defined as empty function if not used (feature is disabled).
  Example:
  

  
              void func_x()
              {
              ...
              #ifdef ROUTE 
               blah = c+a;
               slez = a +z;
              #endif
              ...
              }
  

  
  transformed to:
  

  
              #ifdef ROUTE
                  void optional_init_blah()
                  {
                  blah = c+a;
                  slez = a +z;
                  }
              #else
                  void optional_init_blah(){}
              #endif // ROUTE
          
              void func_x()
              {
              ...
                  optional_init_blah();
              ...
              }
  

  
  In GCC, the optional_init_blah() function will vanish into thin air if ROUTE is not defined. This way, you can isolate the "optional" function to the header file or just place it in the top of the source file to make the code much more readable.
  


• Now, say there's a ROUTE_SUPPORT definition. However, upon examining the code, you found that you don't it in its entirety. Only its static routing that you need. What would you do? Just break the ROUTE_SUPPORT #define to STATIC_ROUTE_SUPPORT and OTHER_ROUTE_SUPPORT_THAT_YOU_DONT_NEED.
  


• Break every very long function to smaller function. Just small enough to fit in your "mental picture". Let's say, nothing more than 150-200 lines of code.
  


• If you found a "cluster" of #define that define consecutive constants and you are sure they all belong to one logical block, it's better to "lump" the constants into an enum. Otherwise, use const int instead of the #define. This way the compiler will help you check for error(s).
  


• Every time you think you grasp the idea behind a function, add a comment. I suggest using a doxygen compatible comment. That way, you can automate or at least semi-automate the documentation for the code you're working with.
  

That's it for now.
This time greetz go to Ayumi Hamasaki for the endless quiet "background sound" that keep me awake.

Ciao.

---

Post a Comment