Home > Articles > Programming > C/C++

  • Print
  • + Share This
Like this article? We recommend

Reworking an Example in C Using Metrics

Let’s review a C code snippet and refactor it following a metrics analysis. I chose C for this example because it’s essentially a legacy language, and the majority of modern programmers won’t be overly familiar with it.

Here’s the original C code:

int main() {
 char* a=(char*)malloc(MEMORY_BLOCK_SIZE);
 memset(a, 'a', MEMORY_BLOCK_SIZE);
 char *b = (char*)malloc(MEMORY_BLOCK_SIZE);
 memset(b, 'b', MEMORY_BLOCK_SIZE);
 char* c = (char*)malloc(MEMORY_BLOCK_SIZE);
 memset(c, 'c', MEMORY_BLOCK_SIZE);
 return 0;
}

Listing 1 shows my first effort at a reworked version of the code. Taking the 11 code metrics as a reference, I’ve tried to fix some of the most basic metrics-related problems: variable naming, spacing, inconsistent parameter-type definition, and exception/error handling.

Listing 1 Reworked post-metrics C code.

int main() {
 char *blockA = (char*)malloc(MEMORY_BLOCK_SIZE);
 if (blockA) {
    memset(blockA, 'a', MEMORY_BLOCK_SIZE);
 }
 char *blockB = (char*)malloc(MEMORY_BLOCK_SIZE);
 if (blockB) {
    memset(blockB, 'b', MEMORY_BLOCK_SIZE);
 }
 char *blockC = (char*)malloc(MEMORY_BLOCK_SIZE);
 if (blockC) {
    memset(blockC, 'c', MEMORY_BLOCK_SIZE);
 }
 return 0;
}

Now let’s see how the first iteration of modified code stacks up against our 11 metrics.

Metric

Result in Modified Code (Listing 1)

Variable names that are too short and provide no usage context

Better. Parameter a is now blockA, b is now blockB, and c is now blockC.

Code that provides no context or obvious purpose

Not much better.

Inconsistent spacing

Better. Notice the use of spaces to improve readability.

Inconsistent parameter-type definition

Better. The pointers all use the same format: char *blockA.

Incomprehensible code

A little better.

Code that’s too long

Not much better.

No exception handling

Better. I now check that the calls to malloc succeeded.

No automatic resource management

Not much better.

External calls that are not contained in separate methods

Not much better.

Inadequate abstraction (need for higher-level code)

Not much better.

Code that’s difficult or impossible to change

Not much better.

We’ve made some progress with four of our metrics in the code. But there’s still scope for improvement. Now let’s make some more minor changes to try to satisfy the metrics more fully. In Listing 2, I introduce a block allocator function to take care of the memory allocations, thereby improving resource management and the level of abstraction (metrics 8–11).

Listing 2 Second attempt at reworked post-metrics C code (adding a block allocator).

char* blockAllocator(int blockSize)
 /* Pass in a required block size and attempt to allocate the memory. If allocation fails, log it and return NULL.*/
{
 char* blockAllocated = (char*)malloc(blockSize);
 if (blockAllocated) {
   return blockAllocated;
 } else {
   /* Log the memory allocation error */
   return NULL;
 }
}

int main() {
 /* Allocate 3 memory blocks for use in caching */
 char* blockA = blockAllocator(MEMORY_BLOCK_SIZE);
 if (blockA) {
   memset(blockA, 'a', MEMORY_BLOCK_SIZE);
  }
  char* blockB = blockAllocator(MEMORY_BLOCK_SIZE);
  if (blockB) {
    memset(blockB, 'b', MEMORY_BLOCK_SIZE);
  }
  char* blockC = blockAllocator(MEMORY_BLOCK_SIZE);
  if (blockC) {
    memset(blockC, 'c', MEMORY_BLOCK_SIZE);
  }
  return 0;
}

After this change, let’s see how the metrics stack up.

Metric

Result in Modified Code (Listing 2)

Variable names that are too short and provide no usage context

Better.

Code that provides no context or obvious purpose

Better.

Inconsistent spacing

Better.

Inconsistent parameter-type definition

Better.

Incomprehensible code

Better.

Code that’s too long

Better.

No exception handling

Better.

No automatic resource management

A little bit better.

External calls that are not contained in separate methods

Better.

Inadequate abstraction (need for higher-level code)

Better.

Code that’s difficult or impossible to change

Better.

The code has improved now that the resource allocations happen in a separate function. In fact, the addition of a block allocator has incorporated nearly all of the metrics. The main code is easier to understand now that resource allocation has been separated into its own function. This change has also helped make the code more specific to solving the business problem, rather than handling memory allocation issues. (This is what I mean by improving the abstraction level.) Also, the code is now more changeable because of the improved separation of concerns. This use of standard design patterns is a big help in improving the code quality.

However, there is still a little more room for improvement; we really should add a block de-allocator function to take care of returning the allocated memory to the free pool. Listing 3 shows a simple example of adding such a block de-allocator function.

Listing 3 Third attempt at reworked post-metrics C code (adding a block de-allocator).

char* blockAllocator(int blockSize)
 /* Pass in a required block size and attempt to allocate the memory. If allocation fails, log it and return NULL.*/
{
 char* blockAllocated = (char*)malloc(blockSize);
 if (blockAllocated) {
   return blockAllocated;
 } else {
   /* Log the memory allocation error */
   return NULL;
 }
}

int blockDeAllocator(char* memoryBlock)
  /* Pass in a block to de-allocate and attempt to de-allocate it. If de-allocation fails, log it and return -1, otherwise return 0.*/
{
 if(memoryBlock) {
   free(memoryBlock);
 } else {
  /* Log the memory attempt to de-allocate a NULL block */
  return -1;
 }
 return 0;
}

int main() {
  /* Allocate memory blocks for use in caching */
  char* blockA = blockAllocator(MEMORY_BLOCK_SIZE);
  if (blockA) {
    memset(blockA, 'a', MEMORY_BLOCK_SIZE);
  }
  char* blockB = blockAllocator(MEMORY_BLOCK_SIZE);
  if (blockB) {
    memset(blockB, 'b', MEMORY_BLOCK_SIZE);
  }
  char* blockC = blockAllocator(MEMORY_BLOCK_SIZE);
  if (blockC) {
    memset(blockC, 'c', MEMORY_BLOCK_SIZE);
  }

  // Lots more code in here

  blockDeAllocator(blockA);
  blockDeAllocator(blockB);
  blockDeAllocator(blockC);

  return 0;
}

What do you think is missing in the calls to blockDeAllocator, in main() in Listing 3? What about a check on the return code? This would be a good idea in order to make certain that the de-allocation occurred as planned. One other thing to add would be to set the block pointers to NULL. This last addition would serve to ensure that the pointers are returned to a known state. With this last set of changes (minus the setting to NULL), the finished code looks like Listing 4.

Listing 4 Final iteration of the reworked post-metrics C code (checking the de-allocator result).

char* blockAllocator(int blockSize)
 /* Pass in a required block size and attempt to allocate the memory. If allocation fails, log it and return NULL.*/
{
 char* blockAllocated = (char*)malloc(blockSize);
 if (blockAllocated) {
   return blockAllocated;
 } else {
   /* Log the memory allocation error */
   return NULL;
 }
}

int blockDeAllocator(char* memoryBlock)
 /* Pass in a block to de-allocate and attempt to de-allocate it. If de-allocation fails, log it and return -1, otherwise return 0.*/
{
 if(memoryBlock) {
   free(memoryBlock);
 } else {
   /* Log the memory attempt to de-allocate a NULL block */
+  return -1;
 }
 return 0;
}

int main() {
  /* Allocate memory blocks for use in caching */
  char* blockA = blockAllocator(MEMORY_BLOCK_SIZE);
  if (blockA) {
    memset(blockA, 'a', MEMORY_BLOCK_SIZE);
  }
  char* blockB = blockAllocator(MEMORY_BLOCK_SIZE);
  if (blockB) {
    memset(blockB, 'b', MEMORY_BLOCK_SIZE);
  }
  char* blockC = blockAllocator(MEMORY_BLOCK_SIZE);
  if (blockC) {
    memset(blockC, 'c', MEMORY_BLOCK_SIZE);
  }

  // Lots more code here

  if (blockDeAllocator(blockA) != 0) {
    cout << "We got an error with blockA" << endl;
  } else {
    cout << "No error with blockA" << endl;
  }
  if (blockDeAllocator(blockB) != 0) {
    cout << "We got an error with blockB" << endl;
  } else {
    cout << "No error with blockB" << endl;
  }
  if (blockDeAllocator(blockC) != 0) {
    cout << "We got an error with blockC" << endl;
  } else {
    cout << "No error with blockC" << endl;
  }
  return 0;
}

Clearly the code in Listing 4 is still not finished! We really should add a logging function to the blockAllocator and blockDeAllocator functions. This change would typically write the results to disk of all allocation and de-allocation operations. Also, we could invoke memset inside the allocator function.

An important point to note is that once the code has been modularized as we’ve just seen, it can then be further improved with minimal risk. In other words, the use of metrics helps to produce a more robust solution, in a nice controlled, iterative fashion.

All this metrics application probably seems like a lot of work. What do we gain from this exercise in metrics consultation and refactoring? Well, for one thing we can now accurately track all memory resource allocations and de-allocations in our original C code. This might not seem to be a big deal on a powerful developer workstation or a large production server. But memory scarcity is often a fact of life on modern embedded platforms such as smartphones or tablet devices. Such machines can run short of memory, and at least now you have the potential for extracting some details if problems occur. The guideline: Memory leaks should always be fixed.

We’ve seen how well metrics work with a difficult legacy language such as C. What about a more modern mainstream language, such as Java?

  • + Share This
  • 🔖 Save To Your Account