As we have posted in the past, refactoring is a very useful technology to help developers become more productive.  I wanted to take a deeper look at how certain refactorings such as “Extract Function/Method” and “Introduce Variable” can be further enhanced with clone detection.

For the focus of this post I will concentrate just on Extract Function/Method.  Say I create some code that I know I will use frequently.  It would make more sense to create a reusable function/method.  Of course I can add a function to my file then pass the proper parameters and function call, or I can click the extract function refactoring which does this automatically for me.

void dosomething (int a, int b)
{
...
   int c = a + b;
   printf("result= %i ",c);
...
}

Now if I select the two lines and run extract function, it will ask me for a name of the function, I’ll call it “add” and we end up with this:

void add (int a, int b)
{
   int c = a + b;
   printf("result= %i ",c);
}

void dosomething (int a, int b)
{
   add(a, b);
}

Same idea for Introduce variable.  Except it works with an expression and replaces it with a variable name of your choice.

So obviously extract function and introduce variable has value,  anything you can do to make the development process more automated can only make you more productive.  But let’s take a look at how we can improve on this and show where you can really take advantage of the power of refactoring.  In most cases you are working with existing code with plenty of duplication.  This is where clone detection comes into place.  Essentially clone detection is the process of finding the same pattern you just highlighted with special attention to code semantics.  This means you now get immediate notification that there are other areas of duplication, even if variable names might be different, and replace with a click of a button.

Taking the simple extract method function, let’s say we have similar code many lines later.

void dosomething(int a, int b)
{
   ...
   int c = a + b;
   printf("result= %i ",c);
   ...
   int d = a + b;
   printf("result= %i ",d);
   ...
}

Now if we run extract method, you get notified about each clone and the choice to replace.

void add(int a, int b)
{
   int c = a + b;
   printf("result= %i ",c);
}

void dosomething(int a, int b)
{
   ...
   add(a, b);
   ...
   add(a, b);
   ...
}

This is a file level refactoring so clone detection can even apply to multiple functions in the same file!   So hopefully with this you will have a new found love for what refactoring can do you.

A recent article on the top five causes of poor software quality and top 5 non-technical mistakes inspired me to also provide a top five on software quality bugs.  Here is my top 5 list of bugs (with some simple examples) that I see time and time again looking at customer code:

1.    Null Pointer dereference

This is far and beyond the most common issue that I see time and time again.

void npd_gen_must() {
int *p = 0; // NULL is assigned.
*p = 1;  // pointer is dereferenced
}

Now this example is pretty basic and if you ever did something this obvious, maybe it was time to re-evaluate your development skills.  The idea is simple, you assign NULL somewhere then dereference it at some point later.  This is usually missed under a complicated control flow (many conditionals).  Or even more common is the fact that I see memory is allocated, but is never checked against NULL.  Now, some organizations don’t care about this but I would hope anyone doing embedded development is all over it.

2.    Null pointer dereference from function

This is really the same thing but with one very important difference.  This deals with issues from functions.

void xstrcpy(char *dst, char *src) {
if (!src) return;
 dst[0] = src[0];
}

char global;

char *xmalloc() {
  if (global) return &global;
  return 0;
}

void npd_func_might(int flag, char *arg) {
  char *p = &arg;
  if (flag) p = xmalloc(); // xmalloc() may return NULL
  if (arg) { p = arg; } // p may get a new value here
  xstrcpy(p, "Hello"); // p will be dereferenced in xstrcpy()
}

It is this inter-procedural (spanning multiple files/functions) context that is often overlooked.

3.    Memory leaks

I have yet to find a programmer in the C/C++ world who doesn’t know this intimately.  Sadly they happen, a lot.

void foobar(int i) {
  char* p = new char[10];
  if(i) {
    p = 0;
  }
  delete[] p;
}

Here we have dynamic memory stored in ‘p’ and allocated through the function ‘new[]‘ at line 3 and is ultimately lost at line 5.

4.    Array index out of bounds

Again, most people know what these are but there are so many variations of this that they are always inevitable.

int main() {
  char fixed_buf[10];
  sprintf(fixed_buf,"Very long format string\n");
  return 0;
}

The string is 24 characters so at line 4 the array index of ‘fixed_buf’ may be out of bounds.

5.    Uninitialized variables

int foo(int t) {
  int x;
  if (t > 16) {
    x = 1;
  } else if (t > 8) {
    x = 2;
  }
  return x + 1;
}

The value of variable ‘x’ can be used at line 8, when it might be uninitialized.  I always found these surprising that these come up as they are pretty basic.  But I tend to only see these in complex control flow paths.  So the developer might check for these under normal conditions but forgot on some path.  Especially for legacy code this might not bite you until you change something later on.

So that’s it.  These examples are pretty simple and certainly not reflective of the real world (or at least I hope not).  Later I will post the same idea for Java code.