patterncMinor
Parsing expressions using idiomatic C
Viewed 0 times
usingidiomaticparsingexpressions
Problem
I'm a C++ programmer using SPOJ problems to learn C(99). This is my solution to the problem CMEXPR, where the goal is to read a valid expression (
The code yields correct answer according to the online judge (got an
```
#include
#include
#include
#include
//#define TESTING
#ifdef TESTING
#define TEST_ASSERT(maCond) do { if (!(maCond)) { printf("%d: %s\n", __LINE__, #maCond); exit(1); } } while (0)
#define TEST_ASSERT_MSG(maCond, maFormat, maMsg) \
do { \
if (!(maCond)) { printf("%d: %s: '" maFormat "'\n", __LINE__, #maCond, (maMsg)); exit(1); } \
} while (0)
#define IF_TESTING(...) __VA_ARGS__
#else
#define TEST_ASSERT(maCond) do {} while (0)
#define TEST_ASSERT_MSG(maCond, maFormat, maMsg) do {} while(0)
#define IF_TESTING(...)
#endif
enum ParsingContext
{
CONTEXT_FIRST_TERM
, CONTEXT_ADDITIVE_EXPRESSION
, CONTEXT_NONFIRST_TERM
};
struct ExprNode
{
char type;
struct ExprNode *child[2];
};
struct NodeMemory
{
struct ExprNode data[MAX_NODE_COUNT];
struct ExprNode *end;
};
struct ExprNode createNode(struct NodeMemory mem, char type)
{
struct ExprNode *node = mem->end++;
TEST_ASSERT(mem->end - mem->data type = type;
IF_TESTING(
node->child[0] = node->child[1] = NULL;
)
return node;
}
struct ExprNode* moveToChild(
struct NodeMemory * restrict mem
, struct ExprNode * restrict node
, size_t idxChild
, char newParentType
)
{
struct ExprNode * result = createNode(mem, newParentType);
result->child[idxChild] = node;
return result;
}
char readChar(const char restrict restrict in)
{
char c = **in;
++*in;
return c;
}
void writeChar(char restrict restrict out, cha
+, -, *, / only) from input and write it on output with superfluous parentheses removed.The code yields correct answer according to the online judge (got an
AC). My question is whether there are any C++-isms in the code and if so, what would be a more idiomatic C way of expressing them. Basically, I don't want to write C++ code in C (just like I don't like C code written in C++).```
#include
#include
#include
#include
//#define TESTING
#ifdef TESTING
#define TEST_ASSERT(maCond) do { if (!(maCond)) { printf("%d: %s\n", __LINE__, #maCond); exit(1); } } while (0)
#define TEST_ASSERT_MSG(maCond, maFormat, maMsg) \
do { \
if (!(maCond)) { printf("%d: %s: '" maFormat "'\n", __LINE__, #maCond, (maMsg)); exit(1); } \
} while (0)
#define IF_TESTING(...) __VA_ARGS__
#else
#define TEST_ASSERT(maCond) do {} while (0)
#define TEST_ASSERT_MSG(maCond, maFormat, maMsg) do {} while(0)
#define IF_TESTING(...)
#endif
enum ParsingContext
{
CONTEXT_FIRST_TERM
, CONTEXT_ADDITIVE_EXPRESSION
, CONTEXT_NONFIRST_TERM
};
struct ExprNode
{
char type;
struct ExprNode *child[2];
};
struct NodeMemory
{
struct ExprNode data[MAX_NODE_COUNT];
struct ExprNode *end;
};
struct ExprNode createNode(struct NodeMemory mem, char type)
{
struct ExprNode *node = mem->end++;
TEST_ASSERT(mem->end - mem->data type = type;
IF_TESTING(
node->child[0] = node->child[1] = NULL;
)
return node;
}
struct ExprNode* moveToChild(
struct NodeMemory * restrict mem
, struct ExprNode * restrict node
, size_t idxChild
, char newParentType
)
{
struct ExprNode * result = createNode(mem, newParentType);
result->child[idxChild] = node;
return result;
}
char readChar(const char restrict restrict in)
{
char c = **in;
++*in;
return c;
}
void writeChar(char restrict restrict out, cha
Solution
These comments are not really related to writing "idiomatic C", merely things I noticed in your code.
-
Your handling of commas in lists is odd:
I have never seen things written like that and don't see any advantage
over the normal use:
-
Your macros and their invocation should allow a semicolon to be placed where
expected. Otherwise, syntax-aware editors can get confused and mess up the
indentation:
Better to move the ; out of the bracket:
-
Your asserts that check against a list of chars could be simpler with
know some people like it that way):
Neater (?):
-
I always find passing double pointers to be awkward and confusing, so I
avoid it if possible. In this case it seems that you could read and write
the input/output string directly to/from stdin/out as you go, replace
-
Note that using an
incorrect. This should be an explicit
-
Your handling of commas in lists is odd:
enum ParsingContext {
CONTEXT_FIRST_TERM
, CONTEXT_ADDITIVE_EXPRESSION
, CONTEXT_NONFIRST_TERM
};
struct ExprNode* moveToChild(
struct NodeMemory * restrict mem
, struct ExprNode * restrict node
, size_t idxChild
, char newParentType
)I have never seen things written like that and don't see any advantage
over the normal use:
enum ParsingContext {
CONTEXT_FIRST_TERM,
CONTEXT_ADDITIVE_EXPRESSION,
CONTEXT_NONFIRST_TERM
};
struct ExprNode* moveToChild(struct NodeMemory * restrict mem,
struct ExprNode * restrict node,
size_t idxChild,
char newParentType)-
Your macros and their invocation should allow a semicolon to be placed where
expected. Otherwise, syntax-aware editors can get confused and mess up the
indentation:
IF_TESTING(
node->child[0] = node->child[1] = NULL;
)
return node;Better to move the ; out of the bracket:
IF_TESTING((node->child[0] = node->child[1] = NULL));
return node;-
Your asserts that check against a list of chars could be simpler with
strchr (also I find the placement of the && here to be distracting, but Iknow some people like it that way):
TEST_ASSERT_MSG(
**in != ')'
&& **in != '+'
&& **in != '-'
&& **in != '*'
&& **in != '/'
&& **in != '\n'
, "%c", **in
);Neater (?):
TEST_ASSERT_MSG(!strchr(")+-*/\n", **in), "%c", **in);-
I always find passing double pointers to be awkward and confusing, so I
avoid it if possible. In this case it seems that you could read and write
the input/output string directly to/from stdin/out as you go, replace
readChar and writeChar with fgetc and fputc and passstdin/stdout around in instead of in and out (or just use thestdin/stdout globals).-
Note that using an
assert to check for exceeding the available memory isincorrect. This should be an explicit
if and exit.TEST_ASSERT(mem->end - mem->data <= MAX_NODE_COUNT);Code Snippets
enum ParsingContext {
CONTEXT_FIRST_TERM
, CONTEXT_ADDITIVE_EXPRESSION
, CONTEXT_NONFIRST_TERM
};
struct ExprNode* moveToChild(
struct NodeMemory * restrict mem
, struct ExprNode * restrict node
, size_t idxChild
, char newParentType
)enum ParsingContext {
CONTEXT_FIRST_TERM,
CONTEXT_ADDITIVE_EXPRESSION,
CONTEXT_NONFIRST_TERM
};
struct ExprNode* moveToChild(struct NodeMemory * restrict mem,
struct ExprNode * restrict node,
size_t idxChild,
char newParentType)IF_TESTING(
node->child[0] = node->child[1] = NULL;
)
return node;IF_TESTING((node->child[0] = node->child[1] = NULL));
return node;TEST_ASSERT_MSG(
**in != ')'
&& **in != '+'
&& **in != '-'
&& **in != '*'
&& **in != '/'
&& **in != '\n'
, "%c", **in
);Context
StackExchange Code Review Q#32040, answer score: 3
Revisions (0)
No revisions yet.