I have a piece of code which looks almost like this (this is simplified just to show the structure):
int Ugly_Calculation(void)
{
type1 *X1 = type1_new();
if (!X1) return 0;
type2 *X2 = type2_new(X1);
if (!X2) return 0;
type3 *X3 = type3_new(X1, X2);
if (!X3) return 0;
int result = PerformCalculation(X1, X2, X3);
free(X3);
free(X2);
free(X1);
return result;
}
It allocates some objects and uses them for some calculation and frees the objects. The typeX_new functions quite often return zero, so the memory is usually not freed correctly.
In my first iteration, I have rewritten the code in following way:
typedef struct
{
type1 *X1;
type2 *X2;
type3 *X3;
} ugly_context;
void free_ctx(ugly_context *ctx)
{
free(ctx->X1);
free(ctx->X2);
free(ctx->X3);
}
int Ugly_Calculation(void)
{
ugly_context ctx = {NULL, NULL, NULL};
ctx->X1 = type1_new();
if (!ctx->X1)
{
free_ctx(&ctx);
return 0;
}
ctx->X2 = type1_new(ctx->X1);
if (!ctx->X2)
{
free_ctx(&ctx);
return 0;
}
ctx->X3 = type1_new(ctx->X1, ctx->X2);
if (!ctx->X3)
{
free_ctx(&ctx);
return 0;
}
int result = PerformCalculation(X1, X2, X3);
free_ctx(&ctx);
return result;
}
My first fix is safe, but it has lost the readability of the original code. My second idea is following:
type2 *type2_new_check(type1 *X1)
{
type2 *result = NULL;
if (X1)
{
result = type2_new(X1);
}
return result;
}
type3 *type3_new_check(type1 *X1, type2 *X2)
{
type3 *result = NULL;
if (X1 && X2)
{
result = type3_new(X1, X2);
}
return result;
}
int PerformCalculation_check(type1 *X1, type2 *X2, type3 *X3)
{
int result = 0;
if (X1 && X2 && X3)
{
result = PerformCalculation(X1, X2, X3);
}
return result;
}
int Ugly_Calculation(void)
{
type1 *X1 = type1_new();
type2 *X2 = type2_new_check(X1);
type3 *X3 = type3_new_check(X1, X2);
int result = PerformCalculation_check(X1, X2, X3);
free(X3);
free(X2);
free(X1);
return result;
}
The code readability is OK and early exits are gone, but it is pretty long for method with 3 memory blocks.
My real code is not 3 allocations, but 15, so the method becomes very creepy and ugly. Is there any nice way, how to solve this pattern without excessive checks and freeing? (The best would be to break the method to smaller parts that are cleanly manageable by checks, but I'd like to avoid that for some reason)
Aucun commentaire:
Enregistrer un commentaire