Alex
Alex

Reputation: 15333

Is there a tool that can refactor this C code correctly?

Lets say I have the following code (the array* function are what we use for resizable arrays and they operate on pointers-to-arrays that are null initialized):

typedef struct MyStruct
{
    int i;
} MyStruct;

MyStruct* GetNewMyStruct(int i)
{
    MyStruct* s = malloc(sizeof(MyStruct));
    s->i = i;
    return s;
}

int SomeFunction(int number, MyStruct *elem)
{
    MyStruct **structs = NULL;
    int i;
    for (i = 0; i < number; i++)
        arrayPush(&structs, GetNewMyStruct(i));
    arrayPush(&structs, elem);
    return arraySize(&structs);
}

I decide that SomeFunction is too large and I want refactor it. Currently where I work we use VisualAssist X, which has some refactoring capabilities, but when I use it on this it does not work correctly. If I attempt to use it to refactor out the loop, this is what I get:

void MyMethod( int number, MyStruct ** structs ) 
{
    int i;
    for (i = 0; i < number; i++)
        arrayPush(&structs, GetNewMyStruct(i));
}

int SomeFunction(int number, MyStruct *elem)
{
    MyStruct **structs = NULL;
    MyMethod(number, structs);
    arrrayPush(&structs, elem);
    return arraySize(&structs);
}

This is not correct. MyMethod should take a MyStruct ***, not a MyStruct **. This is because the code I'm refactoring takes the address of structs. The result is that the refactored version will always return 1 (since only one object has been pushed into my array) rather than number+1. Are there other tools out there that do this type of refactoring correctly?

Upvotes: 1

Views: 186

Answers (1)

dpi
dpi

Reputation: 2007

Eclipse CDT does this correctly (at least the current version Juno). Selecting the declaration of i and the loop and doing Refactor > Extract Function, and setting structs to be an output parameter, produces:

void MyMethod(int number, MyStruct*** structs) {
    int i;
    for (i = 0; i < number; i++)
        arrayPush(&*structs, GetNewMyStruct(i));
}

int SomeFunction(int number, MyStruct *elem)
{
    MyStruct **structs = NULL;
    MyMethod(number, &structs);
    arrayPush(&structs, elem);
    return arraySize(&structs);
}

Upvotes: 1

Related Questions