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

Add UniqueIdentifier field to Expense for Equality checking between Expenses. #42

Merged

Conversation

briyanii
Copy link

@briyanii briyanii commented Sep 23, 2019

#2 #3 #4 #10 #18

Add UniqueIdentifier field to Expense to allow checking of equality between Expenses, replacing Identity fields in the original Person class.

Rename Name class to Description class
Rename 'name' in fields and methods containing the word with 'description'
Note: caused regression in reading json files
Note: Need to fix test cases
Change GUI to show price more clearly
Expenses now determine equality using a randomly generated unique id,
but no response from teachers
add class UniqueIdentifier so that isSameExpense functionality does not change.
Resolve merge conflicts after merge
Add randomly generated UniqueIdentifier to allow checking of equality between Expenses
as Expense does not have Identiy fields like Person
Unit tests which check that success of add command cannot be checked by
comparing with another addcommand instance with the same arguments as equality
of Expense is determined by a unique identifier randomly assigned.

Unit tests which used to check for failiure when editing an expense to have
the same fields as another should no longer fail as the identity of an Expense
is determined by its unique identifier which cannot be edited.
@briyanii briyanii changed the title Refactor person to expense Add UniqueIdentifier field to Expense for Equality checking between Expenses. Sep 26, 2019
@briyanii briyanii requested a review from a team October 2, 2019 18:20
Copy link

@qweiping31415 qweiping31415 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely refactored.

@briyanii briyanii merged commit 57df689 into AY1920S1-CS2103T-T11-1:master Oct 5, 2019
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.

None yet

2 participants