Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

code review for chenhowy part 2 and team #68

Open
yicheng-toh opened this issue Apr 1, 2024 · 0 comments
Open

code review for chenhowy part 2 and team #68

yicheng-toh opened this issue Apr 1, 2024 · 0 comments

Comments

@yicheng-toh
Copy link

yicheng-toh commented Apr 1, 2024

  1. I realised that there are a lot of changes to code ownership. Do consider using these notations to mark out your code.

image

Similar feedback to your peers, there seems to be some form of parsing in this method. Perhaps the parser class can take care of it.
Please take a look at Single Responsibility Principle and cohesion regarding the class design choices.

Perhaps you might want to consider looking at past tp or AB3 for reference.

  1. Similar to Point 2
image

The parser is doing switch statements and also doing error catching. Perhaps you might consider putting it together with the parser? This is so that there would be only one reason to change the functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant