http://codereview.appspot.com/969046/diff/7001/8002 File lily/lexer.ll (right):
http://codereview.appspot.com/969046/diff/7001/8002#newcode545 lily/lexer.ll:545: // loop will be EXPECT_NO_MORE_ARGS. On 2010/05/01 19:56:08, hanwenn wrote:
wouldnt it be clearer to have a function
void translate_markup_signature(SCM predicate_list, vector<int> expect_tokens);
which generates the [expect_scm, expect_scm, expect_markup] (possibly
with
expect_no_more_args) in a vector, and then push the tokens one by one
in the
desired order?
The tokens _are_ pushed one by one in the desired order. So it makes no sense to allocate additional storage just to do the same job.
Also, I think it is better to not use fall-through switches, as they
are a
uncommon and tricky idiom.
Sigh. We have essentially a state machine where the state is determined by the kind of already processed tokens, and depending on the state of the current token, we transition to a new state with different allowed new tokens/transitions. This sort of state machine can usefully only programmed using goto (unless one uses explicit state tables which are even less readable). The switch statement is basically the same as gotoizing a state machine, with the case labels substituting for goto labels. Most state machines (including this one) don't have a structure readily represented in linear structured code. Moving them into a separate procedure does not change that. My changes already make for the bulk of comment lines in this file. So I decided against commenting this even more in violating of the established standards in the rest of the file. Instead I shifted the responsibility of checking the order of arguments to definition time instead of runtime. It has the advantage of triggering prospective errors at a more expected point of time. http://codereview.appspot.com/969046/diff/7001/8002#newcode545 lily/lexer.ll:545: // loop will be EXPECT_NO_MORE_ARGS. On 2010/05/01 19:56:08, hanwenn wrote:
wouldnt it be clearer to have a function
void translate_markup_signature(SCM predicate_list, vector<int> expect_tokens);
which generates the [expect_scm, expect_scm, expect_markup] (possibly
with
expect_no_more_args) in a vector, and then push the tokens one by one
in the
desired order?
Also, I think it is better to not use fall-through switches, as they
are a
uncommon and tricky idiom.
I would expect something like
if(pred == bla("markup?") token = EXPECT_MARKUP; else if (pred == bla("markup-list?") token = EXPECT_MARKUP_LIST; else if (pred == bla("scm?") token = EXPECT_SCM;
Done. http://codereview.appspot.com/969046/show _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel