I made very simple scripting language (interpreter) in C and now, I want to make my code better.
Here is my code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define TRUE (1 == 1)
#define FALSE (1 == 0)
#define FUNCTION_COUNT 2
#define MAX_FUNCTION_NAME_LENGTH 247
#define MAX_FUNCTION_ARG_LENGTH 127
#define MAX_STRING_LENGTH 2048
char *strr(char *str, char *output, int x, int y)
{
int i;
for (i = x + 1; i < y; i++)
{
output[i - (x + 1)] = str[i];
}
return output;
}
int is_string(char *str)
{
if (str[0] == '"' & str[strlen(str) - 1] == '"')
{
return TRUE;
}
else
{
return FALSE;
}
}
int is_sint(char *str)
{
int i = 0;
while (str[i])
{
if (str[i] >= '0' & str[i] <= '9')
{
return TRUE;
}
else
{
return FALSE;
break;
}
}
}
void lang_prints(char *s)
{
char output[MAX_STRING_LENGTH] = "0円";
if (is_string(s))
{
strr(s, output, 0, strlen(s) - 1);
printf("%s\n", output);
}
}
void lang_printi(int i)
{
printf("%d\n", i);
}
int main()
{
int i;
char function_names[FUNCTION_COUNT][MAX_FUNCTION_NAME_LENGTH] = {
"prints",
"printi"
};
char input[MAX_STRING_LENGTH];
char output[MAX_STRING_LENGTH];
char function_name[MAX_FUNCTION_NAME_LENGTH] = "0円";
char function_arg[MAX_FUNCTION_ARG_LENGTH] = "0円";
printf("> ");
scanf("%s", input);
for (i = 0; i < FUNCTION_COUNT; i++)
{
strr(input, function_name, -1, strlen(function_names[i]));
strr(input, function_arg, strlen(function_names[i]), strlen(input) - 1);
if (strcmp(function_name, function_names[i]) == FALSE)
{
if (input[strlen(function_name)] == '(' & input[strlen(function_name) + strlen(function_arg) + 1] == ')')
{
switch (i)
{
case 0:
lang_prints(function_arg);
break;
case 1:
if (is_sint(function_arg))
{
lang_printi(atoi(function_arg));
}
break;
}
}
}
}
}
(Use prints
to print string and printi
to print integer)
1 Answer 1
Careful with choice of identifiers - names beginning with str
are reserved for future library extension. The identifiers beginning with is_
are okay, because _
isn't a lowercase letter.
This is needlessly verbose:
if (str[0] == '"' & str[strlen(str) - 1] == '"') { return TRUE; } else { return FALSE; }
In general, the pattern if (condition) return true; else return false;
can always be replaced by return condition;
:
return str[0] == '"' && str[strlen(str) - 1] == '"';
Note that it's almost always better to use &&
rather than &
for combining logical values.
Similarly:
if (str[i] >= '0' & str[i] <= '9') { return TRUE; } else { return FALSE; break; }
Here the break
is useless, as that line cannot be reached. And the enclosing while
can be just if
, since the function returns on its first iteration. On the other hand, if the while
condition is false, then the function returns without a value - you should get a compilation warning for that.
Also, there's no need to require that str
point to a writeable string, so pass const char*
. And the test looks a lot like isdigit()
(from <ctype.h>
).
I suspect what's really meant is
int is_sint(const char *str)
{
while (*str) {
if (!isdigit((unsigned char)*str++)) { return 0; }
}
return 1;
}
Danger!
scanf("%s", input);
This function takes no account of the capacity of input
, and will happily overwrite data following it if the word it reads is longer. Never use %s
without a specific size for untrusted input. In this code, it's unclear whether you want to read a single word anyway - it looks like a whole line is wanted, for which one should use fgets()
.