# State machine translates text into executable python code [v2]

A few days ago I started writing my own compiler-like program which has the goal to compile plain text into executable python code. After I let my first version review here, I made huge changes to the overall code and added the possibility of adding rectangles. I try to add all the suggested improvements as best as I could, please do not feel offended, if I missed one of your suggestions.

My final goal is to write a program which compiles text into executable python code, which then will display a 3D-Diagram containing all geometric shapes when executed.

At the moment, the user has to provide a text similar to this:

(1.2,45,6)
(7,8,5)
(10,77,88)
(99999,1,1)
R((1,1,1),(2,2,2),(1,3,4))
(5,7,6)
(1,2,3)
(4,5,6)
R((2,3,4),(9,9,9),(3,4,5))
P(2,1,1)


where R(...) symbolizes rectangles and P(...) is an acronym for a profane point. I added the formatting mistake of the last line, purposely, to show that the compiler does not care about spaces. This plain text will be compiled to a python-code similar (depending on the exact preferences of the code) like this.

points =  [
(1.2,45,6),
(7,8,5),
(10,77,88),
(99999,1,1),
]
points.extend([
(5,7,6),
(1,2,3),
(4,5,6),
(2,1,1),
])
rectangels =  [
((1,1,1),(2,2,2),(1,3,4)),
((2,3,4),(9,9,9),(3,4,5)),
]
print(points)
print(rectangels)


For the sake of me, you and all humankind I quickly realized that rewriting the code into a state machine is the proper way to go after suggested by @Edward.

So how could I further improve the following code?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <float.h>
#include <ctype.h>

#define SPACE_FOR_POINTS 4
#define SPACE_FOR_RECTANGLES 4
/*For some reason, maximum size of any double represetend as a string:
StackOverflow: What is the maximum length in chars needed to represent any double value?
*/
#define MAX_DIGITS_DOUBLE 3 + DBL_MANT_DIG - DBL_MIN_EXP
#define MAX_DIGITS_POINTS 5 + 3*MAX_DIGITS_DOUBLE
#define MAX_DIGITS_RECT 17+9*MAX_DIGITS_DOUBLE

void printPointList(char appened, char** points, int index);
void printRectangleList(char appened, char** rectangles, int index);

int main(int argc, char * argv[]) {
FILE* fp = fopen(argv[1], "r");
if(fp == NULL)return errno;

//Do parsing

char** points; //all future points will be stored here
points = malloc(SPACE_FOR_POINTS * sizeof(char*)); //for the moment, make space for SPACE_FOR_POINTS
if(points != NULL){
for(int i = 0; i < SPACE_FOR_POINTS; i++){
/*
This is a point to be stored:
(double,double,double)\0
Calloc is better, because we can later break
creating the list of points/rectangels, when first char is not '(',
which is not necessarily the case for malloc.
*/
points[i] = calloc(MAX_DIGITS_POINTS,1);

if(points[i] == NULL){
fprintf(stderr, "Could not allocate enough memory to perform compilation.");
return -1;
}
}
} else {
fprintf(stderr, "Could not allocate enough memory to perform compilation.");
return -1;
}

char** rectangles; //all future regtangles will be stored here
rectangles = malloc(SPACE_FOR_RECTANGLES*sizeof(char*)); /*for the moment, make space for
SPACE_FOR_RECTANGLES */
if(points != NULL){
for(int i = 0; i < SPACE_FOR_RECTANGLES; i++){
/*
This is a rectangle to be stored:
((double,double,double),(double,double,double),(double,double,double))\0

Calloc is better, because we can later break
creating the list of points/rectangels, when first char is not '(',
which is not necessarily the case for malloc.
*/
rectangles[i] = calloc(MAX_DIGITS_RECT, 1);
if(rectangles[i] == NULL){
fprintf(stderr, "Could not allocate enough memory to perform compilation.");
return -1;
}
}
} else {
fprintf(stderr, "Could not allocate enough memory to perform compilation.");
return -1;
}

char bool_appended_rectangle = 0; //Was a list already printed?
char bool_appended_points = 0; //Was a list already printed?
int numbers_written = 0; //counter
int points_index = -1;
int rectangels_index = -1;
size_t actual_index = 0; //counter

enum{name, openparen, comma, number, endparen, error} state = endparen;
enum{point, rectangle} shape = point;

//State machine
for (char ch = fgetc(fp); ch != EOF; ch = fgetc(fp)) {
if(isspace(ch)){
continue;
}
switch(state){
case name:
if(ch == '('){
state = openparen;
switch (shape) {
case rectangle:
rectangles[rectangels_index][actual_index++] = ch;
break;
case point:
points[points_index][actual_index++] = ch;
break;
default:
break;
}
} else {
state = error;
}
break;
case openparen:
if(ch == '('){
switch (shape) {
case rectangle:
rectangles[rectangels_index][actual_index++] = ch;
state = openparen;
break;
case point:
state = error;
break;
default:
break;
}
}
if(isdigit(ch)){
numbers_written++;
switch (shape) {
case rectangle:
rectangles[rectangels_index][actual_index++] = ch;
state = number;
break;
case point:
points[points_index][actual_index++] = ch;
state = number;
break;
default:
break;
}
}
break;
case number:
if(isdigit(ch)){
switch (shape) {
case rectangle:
rectangles[rectangels_index][actual_index++] = ch;
break;
case point:
points[points_index][actual_index++] = ch;
break;
default:
break;
}
break;
}
if(ch == ','){
switch (shape) {
case rectangle:
if(numbers_written <= 9){
rectangles[rectangels_index][actual_index++] = ch;
state = comma;
} else {
state = error;
}
break;
case point:
if(numbers_written <= 3){
points[points_index][actual_index++] = ch;
state = comma;
} else {
state = error;
}
default:
break;
}
break;
}
if(ch == ')'){
switch (shape) {
case rectangle:
if(numbers_written%3 == 0){
rectangles[rectangels_index][actual_index++] = ch;
state = endparen;
} else {
state = error;
}
break;
case point:
//printf("%d", numbers_written);
if(numbers_written == 3){
state = endparen;
points[points_index][actual_index++] = ch;
} else {
state = error;
}
break;
default:
break;
}
break;
}
if(ch == '.'){
points[points_index][actual_index++] = ch;
break;
}
state = error;
break;
case endparen:
switch (state) {
case point:
points[points_index][actual_index++] = '\0';
break;
case rectangle:
rectangles[rectangels_index][actual_index++] = '\0';
break;
default:
break;
}
if(ch == '(' || ch == 'P'){
if(++points_index >= SPACE_FOR_POINTS){
printPointList(bool_appended_points, points, points_index-1);
bool_appended_points = 1;
//clear all Strings
for(int i = 0; i < points_index; i++){
points[i] = calloc(MAX_DIGITS_POINTS, 1);
}
points_index = 0;
}
}
if(ch == '('){
actual_index = 0;
points[points_index][actual_index++] = ch;
shape = point;
state = openparen;
numbers_written = 0;
break;
}
if(ch == 'P'){
actual_index = 0;
state = name;
shape = point;
numbers_written = 0;
break;
}
if(ch == 'R'){
if(++rectangels_index >= SPACE_FOR_RECTANGLES){
printRectangleList(bool_appended_rectangle, rectangles, rectangels_index-1);
bool_appended_rectangle = 1;
//clear all strings
for(int i = 0; i < rectangels_index; i++){
rectangles[i] = calloc(MAX_DIGITS_RECT, 1);
}
rectangels_index = 0;
}
actual_index = 0;
state = name;
shape = rectangle;
numbers_written = 0;
break;
}
if(ch == ','){
if(shape == rectangle){
if(numbers_written < 9){
rectangles[rectangels_index][actual_index++] = ch;
state = comma;
} else {
state = error;
}
}
break;
}
if(ch == ')'){
if(shape == rectangle){
rectangles[rectangels_index][actual_index++] = ch;
rectangles[rectangels_index][actual_index++] = '\0';
break;
}
}
state = error;
break;
case comma:
switch (shape) {
case point:
if(isdigit(ch)){
numbers_written++;
state = number;
points[points_index][actual_index++] = ch;
break;
}
state = error;
break;
case rectangle:
if(isdigit(ch)){
numbers_written++;
rectangles[rectangels_index][actual_index++] = ch;
state = number;
break;
}
if(ch == '('){
rectangles[rectangels_index][actual_index++] = ch;
state = openparen;
break;
}
state = error;
break;
default:
break;
}
break;
default:
fprintf(stderr, "Error: Corrupted File.");
return -1;
}
}

printPointList(bool_appended_points, points, points_index);
printRectangleList(bool_appended_rectangle, rectangles, rectangels_index);
printf("print(points)\n");
printf("print(rectangels)\n");

fclose(stdout); //needed?
fclose(fp);
return 1;
}
void printPointList(char appended, char** points, int index){
if(!appended){
appended = 1;
printf("points =  [\n");
} else {
printf("points.extend(\n");
}
for(int i = 0; i <= index; i++){
printf("%s,\n", points[i]);
}
if(!appended){
printf("]\n");
} else {
printf("])\n");
}
}
void printRectangleList(char appended, char** rectangles, int index){
if(!appended){
appended = 1;
printf("rectangle =  [\n");
} else {
printf("rectangle.extend(\n");
}
for(int i = 0; i <= index; i++){
printf("%s,\n", rectangles[i]);
}
if(!appended){
printf("]\n");
} else {
printf("])\n");
}
}

• Maybe I'm mistaken but I think this could be 2 separate questions that would target two very different set of reviewing skills. You have C code that translates text into python code, which will then print shapes? – IEatBagels Jun 21 '19 at 14:09
• You assume correctly. I only care for the C code. – TVSuchty Jun 21 '19 at 16:37
• Or at least not so much about the python code. – TVSuchty Jun 24 '19 at 10:37
• Is there any reason for points.extend at all? – klutt Jun 24 '19 at 16:25
• Sure, writing everything in one list makes it unreadable. So I thought chunking would sustain some readability... – TVSuchty Jun 24 '19 at 20:52

## Choice of language

I understand you probably want to do this in C, but I felt it was worth mentioning that Python is a much more suitable language for this task.

## malloc call

You're using points = malloc(SPACE_FOR_POINTS * sizeof(char*)). I'll advice you to instead use points = malloc(SPACE_FOR_POINTS * sizeof(*points)) to prevent problems if you change type of points in the future.

## Comparing pointers to NULL

Not a big deal, but checking if a pointer is valid with if(ptr) or not valid with if(!ptr) is so common in C that it adds almost no readability.

## Comments

You're using some unmotivated comments. Like when you are wasting 7(!) lines just to explain why you are using calloc instead of malloc.

## return instead of exit

Using return instead of exit works as long as you're in the main function. So if you have a return statement that should quit the program, then instead use exit(EXIT_FAILURE) so that it does not matter if you refactor it into a function.

## Massive main function

The thing that strikes me most is that you have a massive main function with a really huge switch statement. The first thing I would do put the whole switch statement into a function with the signature enum state DFA(enum state state, char ch) and then have a main loop like this:

for (char ch = fgetc(fp); ch != EOF; ch = fgetc(fp)) {
state = DFA(state, ch);
if(state == error) { /* Handle error*/ }
}


But this would not be enough, since your states are much more complicated than that. The actual state of your machine is a combination of state and shape. So I would rename the current state. I will choose category but I'm sure you can come up with something better. Then I would do this:

struct state {
enum category;
enum shape;
};

struct state DFA(struct state state, char ch);


Of course, you would also need to pass rectangles, points, rectangles_index, points_index and actual_index somehow. I would use a struct for that:

struct data {
char **rectangles;
char **points;
size_t rectangle_index;
size_t points_index;
size_t actual_index;
};

struct state DFA(struct state state, char ch, struct data * data);


Another alternative would be to just declare them as globals. Don't think that would be a too terrible idea in your project.

## Massive switch

I would consider rewriting the whole automata. I would have rewritten it a bit like this:

switch(state) {
case name:
nameFunction(ch, data); break;
case openparen:
openparenFunction(ch, data); break;

• Hey thank you for your suggestion; I also considered reducing the size of main but was not sure about the concrete design-choice. My goal is indeed writing this in C, so I learn C. In Python, I would have considered this way to easy, since there are fundamentals I do not have to care about. I think I will create more complex compiler-exercises in python in the future and hope for your review :) – TVSuchty Jun 24 '19 at 10:36
• Why would you not pass pointers into the DFA and return void? What does state mean? – TVSuchty Jun 27 '19 at 9:04
• Okay, while refactoring I figured out why. Because you have to dereference tons of time! – TVSuchty Jun 27 '19 at 9:30