3
\$\begingroup\$

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)

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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().

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.