Wordy’s tests expect the implementation to make a distinction between two kind of errors:
Syntax error - any bad combination of valid numbers and valid operations or empty expression
Unknown operation - any unexpected text
This distinction is not useful and favors a particular class of solutions where the lexical tokens are first extracted (failure leads to “Unknown operation”) and then the expression is parsed (and failure here leads to “Syntax error”).
This is not the only valid approach for parsing this toy syntax, but other solutions might not distinguish these two failure cases. With some trickery the tests can probably be satisfied resulting in ugly solution code.
Please consider testing that the code just throws anything on bad input without imposing this distinction.
The tangible improvement that this change makes (ideally with code examples of what your change enables/stops).
I have chosen a very simple approach to solving the exercise: I’m iterating over partial expressions with a single regular expression narrowing the expression after each iteration to the “rest” part:
// unnecessary complicated in order to distinguish "Unknonw operation" error from "Syntax error"
const QUESTION_WITH_VALID_TOKENS =
/^What is(?<expr>( (-?\d+|plus|minus|multiplied by|divided by))*)\?$/;
const PARTIAL_EXPRESSION =
/^(?<num>-?\d+)( (?<op>plus|minus|multiplied by|divided by) (?<rest>.+))?$/;
type Op = (num: number) => void;
/**
* Extract expression from the question with some premature validation
*
* @param question the whole quesion
* @returns expression part of the question
*/
function expression(question: string): string {
const match = question.match(QUESTION_WITH_VALID_TOKENS);
if (!match || !match.groups) throw Error("Unknown operation");
// trim because the expr capture group includes a leading space
return match.groups.expr.trim();
}
export function answer(question: string): number {
let expr = expression(question);
let answer = 0;
// the first op is mere assignment
let op: Op = (num: number) => (answer = num);
const ops: Record<string, Op> = {
plus: (num: number) => (answer += num),
minus: (num: number) => (answer -= num),
"multiplied by": (num: number) => (answer *= num),
"divided by": (num: number) => (answer /= num),
};
while (true) {
const match = expr.match(PARTIAL_EXPRESSION);
if (!match || !match.groups) throw Error("Syntax error");
// if match.groups not empty there is non-optional num
let num = Number.parseInt(match.groups.num);
op(num);
// if optional op is missing then the expression is complete
if (!match.groups.op) break;
// remember next op
op = ops[match.groups.op];
// narrow the expression to the remaining bit
expr = match.groups.rest;
}
return answer;
}
The first regular expression is unnecessarily complex just in order to distinguish “Unknown operation” and “Syntax error” cases.
The impact on existing solutions, and why this is worth it.
A backwards compatible approach would be to have the two tests that expect “Unknown operation” expect any of the two error values. This way old passing solutions will remain passing, but new solutions could benefit from the change.
Any changes to the concepts taught or used.
I think it’s a practice exercise that doesn’t explicitly teaches any particular concepts.
An understanding of why things might be the way they are.
I think that the distinction is due to assumption about the irrelevant implementation details. I might be wrong though.
Most of the wordy exercise is satisfying to complete, but getting the errors correct at the end is very frustrating. The errors themselves seem organised in a non sensical way to me, and getting the tests to pass involves making the code worse, which seems like a bad lesson for a student.
This has typically been my approach when porting this specific exercise to other tracks, e.g. emacs-lisp
As for changing existing tracks, I’ll defer to others.
The description further makes clear that the goal is to “parse” the statement and deal with different kind of errors:
Parse and evaluate simple math word problems returning the answer as an integer.
And
Iteration 4 — Errors
The parser should reject:
Unsupported operations (“What is 52 cubed?”)
Non-math questions (“Who is the President of the United States”)
Word problems with invalid syntax (“What is 1 plus plus 2?”)
My opinion on these older exercises is, in general, pretty indifferent, but I do take part in the discussions, especially since I maintain JS and TS tracks. Always willing to listen, however please be aware that there is a lot of time spent in the past thinking about these things. This means that when you say something like:
I have written my fair share of parsers, lexers, compilers, syntax highligthers and static analysis libraries and from that point of view, because this exercise was originally meant to be a parsing exercise, the input validation here, and thus the shoehorning the error types as well as not allowing specific kind of solutions is not unwarranted.
begin
yield serializer.serialize(item, media_type, context: routes),
serializer: serializer,
media_type: media_type,
emit: item_emit_style
rescue Serialization::OutputValidationFailedError => e
raise ConfigurationError.new("Validation failed for #{serializer.name} serializing #{routes.safe_path_for(item)} to #{media_type}:\n#{e.message}")
rescue ArgumentError => e
raise e # rewrite stack trace
end
end
From production code (Ruby), there is a distinct difference between “I set this up incorrectly” and “the data is set up correctly but invalid” and “unexpected error occurs”.
In the hypothetical example of Exercism’ exercise: yeah no there is really no point. I agree. But given that this a parser exercise, the idea of the distinction does make sense.
At this moment I am not convinced JavaScript and TypeScript should change this particular exercise, but in general I do agree that arbitrary errors for arbitrary input validation are a bad idea (which in this case, …is not applicable).
But given that this a parser exercise, the idea of the distinction does make sense.
In my opinion “Parse and evaluate simple math word problems” does not imply two stage parser that does lexical analysis up front. If that were the concept the exercise were teaching, it would be explained in the readme. JS/TS make it really easy to solve this problem without separate lexical analysis stage and the syntax of the word problems lends itself naturally to this simpler class of solutions. Every community solution I had a look at chooses a simple single pass lexer-parser approach but is forced to detect the “Unknown operation” errors by the test suite.
I agree that it should. Unfortunately we tried to be extremely summarised when those were added 2018 and before.
JavaScript and TypeScript students in particular take shortcuts all the time. We have to often resort to extreme measures for people to follow the instructions to begin with. Some examples:
This means that looking at the lowest common denominator (the community solutions) isn’t necessarily a good guide to what something is supposed to be. However, it does usually indicate something is lacking (either the instructions/readme being inadequate, or exercise tests missing).
I would be happy to take a clarifying instructions.appends.md for this exercise if that clarifies things, but I do not think this particular exercise warrants an error downgrade “just so more solutions pass”. If we get a proper lexer/parser exercise, I would reconsider that.
If the problem specifications are updated to make that change across the board, I will defer to that decision instead.
I add another way to think about this: The requirements are to be fulfilled by the program, not adjusted to an “easy” solution.
In this case, the requirements are to give distinct feedback on error classes (which make sense for parsing applications). So every solution must deliver all required feedback. If I find, that my solution does not easily do that, I should think about how to redesign my solution.
Lowering the bar here does not make any sense to me.