feat: add jack parser - Githubissues

nandtotetris / computer-visualizer

0 stars 0 forks source link

feat: add jack parser #47

Closed henok-tesfaye closed 3 years ago

henok-tesfaye commented 3 years ago

Make progress on #32

What kind of change does this PR introduce? It's a feature PR which adds a complete jack parser that can handle expressions as well.

Does this PR introduce a breaking change? No

What needs to be documented once your changes are merged? Nothing

mikerezene commented 3 years ago

It would be great if we all follow the API that is recommended in the book. i found it that the API must look like this api_suggested

henok-outlier commented 3 years ago

@mikereze we followed the API used in the book. Did you see anything that opposes the API?

mikerezene commented 3 years ago

@mikereze we followed the API used in the book. Did you see anything that opposes the API?

I'm not sure if it is mentioned in the book or not but I saw eat() that noam suggested. In addition I didn't saw a command like tokenizer.back() in the book.

henok-outlier commented 3 years ago

@mikereze that's an implementation detail instead of an API.

mikerezene commented 3 years ago

@mikereze that's an implementation instead of an API.

which one?

mikerezene commented 3 years ago

tobeChecked2

it is a bit simpler if we use the eat() command so that the code will be concise and simple to read

tobeChecked1

henok-tesfaye commented 3 years ago

@mikereze API is all about the function inputs and its outputs. But you are suggesting to copy paste an already implemented logic.

mezzzi commented 3 years ago

it is a bit simpler if we use the eat() command so that the code will be concise and simple to read

@mikereze good suggestion. But at the end of the day implementation detail will have to be left up to the developer, and @henok-tesfaye is justified to go with his own style of implementation as long as he is meeting the api requirements. And we cannot force him to use the eat or the drink method, whatever the book authors chose to use as an example.

And given the limited time we have, we shouldn't bother much about following a certain narrow implementation approach, it is enough if we catch obvious errors, or logic flaws, in our reviews.

mikerezene commented 3 years ago

@mikereze API is all about the function inputs and its outputs. But you are suggesting to copy paste an already implemented logic.

@henok-tesfaye @mezzzi you are right. everything seems great to me