patterncMinor
Template system for FastCGI app in C
Viewed 0 times
fastcgitemplateappsystemfor
Problem
I'm doing a small FastCGI application in C and I've created this template system to keep the design away from the code.
There's a binary tree containing all possible first characters, each node points to another binary tree containing all characters that can appear after it. Some nodes have the replacement, when they are reached it means that a match was found.
The template system will read the characters from the file and parse the tree at the same time, always favoring the longest match.
I would like to know how to improve efficiency and reduce complexity.
word_tree.h
word_tree.c
```
#include "word_tree.h"
#include
void word_tree_init(Word_Tree **root)
{
*root = NULL;
}
static Word_Tree *word_tree_new(void)
{
Word_Tree *wt = malloc(sizeof(Word_Tree));
if(wt == NULL)
return NULL;
wt->sequence = NULL;
wt->smaller = NULL;
wt->greater = NULL;
wt->replacement = NULL;
wt->c = '\0';
return wt;
}
void word_tree_delete(Word_Tree *root)
{
if(root->sequence)
word_tree_delete(root->sequence);
if(root->smaller)
word_tree_delete(root->smaller);
if(root->greater)
word_tree_delete(root->greater);
free(root);
}
WT_Return word_tree_insert(Word_Tree **root, const char word, const char replacement)
{
int comparison;
//Signal that the replacement should be inserted
if(*word == '\0')
return WT_DONE;
There's a binary tree containing all possible first characters, each node points to another binary tree containing all characters that can appear after it. Some nodes have the replacement, when they are reached it means that a match was found.
The template system will read the characters from the file and parse the tree at the same time, always favoring the longest match.
I would like to know how to improve efficiency and reduce complexity.
word_tree.h
#ifndef WORD_TREE_H
#define WORD_TREE_H
typedef enum {
WT_SUCCESS, WT_ERROR, WT_DUPLICATE, WT_DONE //WT_DONE is for internal use only
} WT_Return;
typedef struct Word_Tree{
struct Word_Tree *sequence;
struct Word_Tree *smaller;
struct Word_Tree *greater;
const char *replacement;
int c;
} Word_Tree;
void word_tree_init(Word_Tree **root);
void word_tree_delete(Word_Tree *root);
WT_Return word_tree_insert(Word_Tree **root, const char *word, const char *replacement);
const Word_Tree *word_tree_find(const Word_Tree *wt, int c);
#endifword_tree.c
```
#include "word_tree.h"
#include
void word_tree_init(Word_Tree **root)
{
*root = NULL;
}
static Word_Tree *word_tree_new(void)
{
Word_Tree *wt = malloc(sizeof(Word_Tree));
if(wt == NULL)
return NULL;
wt->sequence = NULL;
wt->smaller = NULL;
wt->greater = NULL;
wt->replacement = NULL;
wt->c = '\0';
return wt;
}
void word_tree_delete(Word_Tree *root)
{
if(root->sequence)
word_tree_delete(root->sequence);
if(root->smaller)
word_tree_delete(root->smaller);
if(root->greater)
word_tree_delete(root->greater);
free(root);
}
WT_Return word_tree_insert(Word_Tree **root, const char word, const char replacement)
{
int comparison;
//Signal that the replacement should be inserted
if(*word == '\0')
return WT_DONE;
Solution
A couple notes:
-
This looks fishy to me:
It seems like a very complex system when you mostly just return the
case values (except for one specific value). Here is what I would do
to clean it up (untested):
-
You have a comment here:
I think that you should be using
-
This looks fishy to me:
//Try to find the character
if(comparison > 0){
switch(word_tree_insert(&(*root)->greater, word, replacement)){
case WT_DUPLICATE:
return WT_DUPLICATE;
case WT_ERROR:
return WT_ERROR;
case WT_SUCCESS:
return WT_SUCCESS;
case WT_DONE:
if((*root)->replacement != NULL)
return WT_DUPLICATE;
(*root)->replacement = replacement;
return WT_SUCCESS;
}
}It seems like a very complex system when you mostly just return the
case values (except for one specific value). Here is what I would do
to clean it up (untested):
//Try to find the character
if(comparison > 0)
{
WT_Return val = word_tree_insert(&(*root)->greater, word, replacement);
if(val != WT_DONE) return val;
else
{
if((*root)->replacement) return WT_DUPLICATE;
(*root)->replacement = replacement;
return WT_SUCCESS;
}
}-
You have a comment here:
//If we are inserting, it's impossible to be a duplicateI think that you should be using
assert() in this one situation to verify that, since it is used to check for situations that "can't happen" or are "impossible".Code Snippets
//Try to find the character
if(comparison > 0){
switch(word_tree_insert(&(*root)->greater, word, replacement)){
case WT_DUPLICATE:
return WT_DUPLICATE;
case WT_ERROR:
return WT_ERROR;
case WT_SUCCESS:
return WT_SUCCESS;
case WT_DONE:
if((*root)->replacement != NULL)
return WT_DUPLICATE;
(*root)->replacement = replacement;
return WT_SUCCESS;
}
}//Try to find the character
if(comparison > 0)
{
WT_Return val = word_tree_insert(&(*root)->greater, word, replacement);
if(val != WT_DONE) return val;
else
{
if((*root)->replacement) return WT_DUPLICATE;
(*root)->replacement = replacement;
return WT_SUCCESS;
}
}//If we are inserting, it's impossible to be a duplicateContext
StackExchange Code Review Q#54678, answer score: 6
Revisions (0)
No revisions yet.