Skip to content

Xiulan's Homework #25

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Xiulanshi
Copy link

No description provided.

@Mataniko
Copy link

Hey Xiulan,

Great work here! You did a really good job writing very concise code and separating your Model into a separate class file.

You added an index property on the CQCategory which wasn't part of the requirements, and is also not really used. It's not a big deal when working by yourself on an small app, but when you start working with a team, following requirements and making sure there's no redundancies becomes more important.

@Xiulanshi
Copy link
Author

Hi Matan,
Thank you for looking into my homework.
You are right that the index property isn't used much for now. Because Mike said we could do more than just meet the requirement. I was thinking of to allow user to add more categories, so I added it. Though I didn't have enough time to finish the add category functionality. ;-) Thanks again.

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

Successfully merging this pull request may close these issues.

2 participants