Saturday, January 25, 2014

Practical Style

In regard to my last post about the importance of a consistent code style in a VCS environment, I wanted to post a concrete example.

Consider the following code,


#include <stdio.h>
#include <stdlib.h>

int
printMessage(char* message);

int
getName(char* message);

void
checkExit(int code);

int
getName(char* message)
{
  char* name = malloc(sizeof(char)*10);
  int i = 0;
  int err = 0;
  char c = 0;
  while((c = getchar()) != '\n')
    {
      name[i] = c;
      ++i;
    }
  err = printf("%s %s\n", message, name);
  free(name);
  return err;
}

int
printMessage(char* message)
{
  return printf("%s\n", message);
}

void
checkExit(int code)
{
  if(code < 0)
    exit(code);
  else
    return;
}

int
main(int argc, char* argv[])
{
  int err = printMessage("What is your name?");
  checkExit(err);
  err = getName("Hello,");
  checkExit(err);
  return 0;
}
This is a simple little C program that will read in your name and print it back at you. It does have one issue (at least, I wrote it rather quickly). In the function getName function there is no check to make sure you aren't reading over the end of the buffer (In C you must worry about such primitive things!). Now, let's say another programmer with a different sense of style comes in to fix this later, but he also changes the code to match his personal style preferences, no camel-case, function return types on the same line as the function name, different feelings about braces, etc. Below are his changes.


#include <stdio.h>
#include <stdlib.h>

int printmessage(char* message);

int getname(char* message);

void checkexit(int code);

int getname(char* message) {
  char* name = malloc(sizeof(char)*10);
  int i = 0;
  int err = 0;
  char c = 0;
  for(; i < 10 && (c = getchar()) != '\n'; i++)
    {
      name[i] = c;
    }
  err = printf("%s %s\n", message, name);
  free(name);
  return err;
}

int printmessage(char* message) {
  return printf("%s\n", message);
}

void checkexit(int code) {
  if(code < 0)
    exit(code);
  else
    return;
}

int main(int argc, char* argv[]) {
  int err = printmessage("What is your name?");
  checkexit(err);
  err = getname("Hello,");
  checkexit(err);
  return 0;
}

Now semantically are identical, except for fixing that buffer length check in getname (formally "getName"), but look at the diff that is produced from them.


4c4,5
< int printmessage(char* message);
---
> int
> printMessage(char* message);
6c7,8
< int getname(char* message);
---
> int
> getName(char* message);
8c10,11
< void checkexit(int code);
---
> void
> checkExit(int code);
10c13,15
< int getname(char* message) {
---
> int
> getName(char* message)
> {
15c20
<   for(; i < 10 && (c = getchar()) != '\n'; i++)
---
>   while((c = getchar()) != '\n')
17a23
>       ++i;
24c30,32
< int printmessage(char* message) {
---
> int
> printMessage(char* message)
> {
28c36,38
< void checkexit(int code) {
---
> void
> checkExit(int code)
> {
35,39c45,51
< int main(int argc, char* argv[]) {
<   int err = printmessage("What is your name?");
<   checkexit(err);
<   err = getname("Hello,");
<   checkexit(err);
---
> int
> main(int argc, char* argv[])
> {
>   int err = printMessage("What is your name?");
>   checkExit(err);
>   err = getName("Hello,");
>   checkExit(err);

Whoa! That is a large diff for such a small semantic change! Take a look at the diff that would have been produced if the style had not changed.

20c20
<   for(; i < 10 && (c = getchar()) != '\n'; i++)
---
>   while((c = getchar()) != '\n')
22a23
>       ++i;
A diff like this is much more clear.

Hopefully this illustrates why choosing and sticking to a particular code style is important in a VCS environment.

No comments:

Post a Comment