-
Notifications
You must be signed in to change notification settings - Fork 5
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
Refactor person to contact #27
Refactor person to contact #27
Conversation
It appears that the JsonSerializableAddressBook class cannot be refactored without breaking a lot of things. |
I believe this is because that class (and other similar ones) functions as the interface between the final JSON file stored and the internal models, converting to and from via Jackson. Since many tests use template JSON files to verify JSON Serializable class behaviors, all JSON test cases must be manually modified and likely cannot be automatically refactored by the IDE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me. The changes pass the tests on my side as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay. We may need to change the name of the person package to contact later on, but for now it should be fine!
Updated a few test cases and documentation to reflect the change and changed
toString
method to pass test cases.