-
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
Update Sort to include attribute Faculty #89
Changes from all commits
fc9c7b0
b0a3c3e
0ad0515
863585d
1f6d93b
2e6a443
d4d5f8b
b79fc90
2fb1e20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package staffconnect.model.person.comparators; | ||
|
||
import java.util.Comparator; | ||
|
||
import staffconnect.model.person.Person; | ||
|
||
|
||
/** | ||
* Represents a Comparator for a Person's Venue in the staff book. | ||
*/ | ||
public class FacultyComparator implements Comparator<Person> { | ||
|
||
public static final FacultyComparator FACULTY_COMPARATOR = new FacultyComparator(); | ||
|
||
@Override | ||
public int compare(Person p1, Person p2) { | ||
return p1.getFaculty().toString().compareToIgnoreCase(p2.getFaculty().toString()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,5 +90,23 @@ | |
"venue" : "underworld avenue", | ||
"tags" : [ "classmate" ], | ||
"availabilities" : [ "FRIDAY" ] | ||
}, { | ||
"name" : "Leonardo DiCaprio", | ||
"phone" : "88472234", | ||
"email" : "[email protected]", | ||
"module": "TS2237", | ||
"faculty": "Arts and Social Sciences", | ||
"venue" : "LT13", | ||
"tags" : [ ], | ||
"availabilities" : [ ] | ||
}, { | ||
"name" : "Michael Jackson", | ||
"phone" : "92347123", | ||
"email" : "[email protected]", | ||
"module": "MUA1163", | ||
"faculty": "Music", | ||
"venue" : "YSTCM-SR9", | ||
"tags" : [ ], | ||
"availabilities" : [ ] | ||
} ] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,14 @@ | |
|
||
|
||
import static staffconnect.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; | ||
import static staffconnect.logic.parser.CliSyntax.PREFIX_FACULTY; | ||
import static staffconnect.logic.parser.CliSyntax.PREFIX_MODULE; | ||
import static staffconnect.logic.parser.CliSyntax.PREFIX_NAME; | ||
import static staffconnect.logic.parser.CliSyntax.PREFIX_PHONE; | ||
import static staffconnect.logic.parser.CliSyntax.PREFIX_VENUE; | ||
import static staffconnect.logic.parser.CommandParserTestUtil.assertParseFailure; | ||
import static staffconnect.logic.parser.CommandParserTestUtil.assertParseSuccess; | ||
import static staffconnect.model.person.comparators.FacultyComparator.FACULTY_COMPARATOR; | ||
import static staffconnect.model.person.comparators.ModuleComparator.MODULE_COMPARATOR; | ||
import static staffconnect.model.person.comparators.NameComparator.NAME_COMPARATOR; | ||
import static staffconnect.model.person.comparators.PhoneComparator.PHONE_COMPARATOR; | ||
|
@@ -56,9 +58,10 @@ public void parse_validArgs_returnsSortCommand() { | |
@Test | ||
public void parse_validArgs_returnsSortCorrectAttribute() { | ||
|
||
assertParseSuccess(parser, "" + PREFIX_MODULE, new SortCommand(MODULE_COMPARATOR)); | ||
assertParseSuccess(parser, "" + PREFIX_NAME, new SortCommand(NAME_COMPARATOR)); | ||
assertParseSuccess(parser, "" + PREFIX_PHONE, new SortCommand(PHONE_COMPARATOR)); | ||
assertParseSuccess(parser, "" + PREFIX_MODULE, new SortCommand(MODULE_COMPARATOR)); | ||
assertParseSuccess(parser, "" + PREFIX_FACULTY, new SortCommand(FACULTY_COMPARATOR)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it would be better to arrange in alphabetic order |
||
assertParseSuccess(parser, "" + PREFIX_VENUE, new SortCommand(VENUE_COMPARATOR)); | ||
|
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package staffconnect.model.person.comparators; | ||
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertNotEquals; | ||
import static org.junit.jupiter.api.Assertions.assertTrue; | ||
import static staffconnect.model.person.comparators.FacultyComparator.FACULTY_COMPARATOR; | ||
import static staffconnect.model.person.comparators.ModuleComparator.MODULE_COMPARATOR; | ||
import static staffconnect.model.person.comparators.NameComparator.NAME_COMPARATOR; | ||
import static staffconnect.model.person.comparators.PhoneComparator.PHONE_COMPARATOR; | ||
import static staffconnect.model.person.comparators.VenueComparator.VENUE_COMPARATOR; | ||
import static staffconnect.testutil.TypicalPersons.ALICE; | ||
import static staffconnect.testutil.TypicalPersons.LEONARDO; | ||
import static staffconnect.testutil.TypicalPersons.MICHAEL; | ||
|
||
import org.junit.jupiter.api.Test; | ||
|
||
public class FacultyComparatorTest { | ||
@Test | ||
public void doesNotEquals() { | ||
assertNotEquals(FACULTY_COMPARATOR, NAME_COMPARATOR); | ||
assertNotEquals(FACULTY_COMPARATOR, PHONE_COMPARATOR); | ||
assertNotEquals(FACULTY_COMPARATOR, MODULE_COMPARATOR); | ||
assertNotEquals(FACULTY_COMPARATOR, VENUE_COMPARATOR); | ||
} | ||
|
||
@Test | ||
public void checkComparator() { | ||
|
||
assertTrue(FACULTY_COMPARATOR.compare(ALICE, MICHAEL) <= -1); // Computing < Music | ||
assertTrue(FACULTY_COMPARATOR.compare(MICHAEL, ALICE) >= 1); // Music > Computing | ||
|
||
assertTrue(FACULTY_COMPARATOR.compare(LEONARDO, MICHAEL) <= -1); // Arts and Social Sciences < Music | ||
assertTrue(FACULTY_COMPARATOR.compare(MICHAEL, LEONARDO) >= 1); // Music > Arts and Social Sciences | ||
|
||
assertEquals(FACULTY_COMPARATOR.compare(LEONARDO, LEONARDO), 0); | ||
// Arts and Social Sciences = Arts and Social Sciences | ||
assertEquals(FACULTY_COMPARATOR.compare(MICHAEL, MICHAEL), 0); // Music = Music | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,18 +52,11 @@ public class TypicalPersons { | |
.withPhone("9482224").withEmail("[email protected]").withModule("CS2100") | ||
.withFaculty("Computing").withVenue("michegan ave").build(); | ||
public static final Person FIONA = new PersonBuilder().withName("Fiona Kunz").withPhone("9482427") | ||
.withEmail("[email protected]").withVenue("little tokyo") | ||
.withFaculty("Computing").withModule("CS2101").build(); | ||
.withEmail("[email protected]").withModule("CS2101") | ||
.withFaculty("Computing").withVenue("little tokyo").build(); | ||
public static final Person GEORGE = new PersonBuilder().withName("George Best").withPhone("9482442") | ||
.withEmail("[email protected]").withVenue("4th street") | ||
.withFaculty("Computing").withModule("CS2102").build(); | ||
public static final Person KAFKA = new PersonBuilder().withName("Kafka Apache").withPhone("9452413") | ||
.withEmail("[email protected]").withModule("CS2102").withFaculty("Business") | ||
.withVenue("pteruges avenue").withTags("classmate").withAvailabilities("FRIDAY").build(); | ||
public static final Person NATASHA = new PersonBuilder().withName("Natasha Harrower").withPhone("8019394") | ||
.withEmail("[email protected]").withModule("CS2102") | ||
.withFaculty("Computing").withVenue("underworld avenue") | ||
.withTags("classmate").withAvailabilities("FRIDAY").build(); | ||
.withEmail("[email protected]").withModule("CS2102") | ||
.withFaculty("Computing").withVenue("4th street").build(); | ||
|
||
// Manually added | ||
public static final Person HOON = new PersonBuilder().withName("Hoon Meier").withPhone("8482424") | ||
|
@@ -73,6 +66,20 @@ public class TypicalPersons { | |
.withEmail("[email protected]").withModule("CS2103T") | ||
.withFaculty("Computing").withVenue("chicago ave").build(); | ||
|
||
public static final Person KAFKA = new PersonBuilder().withName("Kafka Apache").withPhone("9452413") | ||
.withEmail("[email protected]").withModule("CS2102").withFaculty("Business") | ||
.withVenue("pteruges avenue").withTags("classmate").withAvailabilities("FRIDAY").build(); | ||
public static final Person NATASHA = new PersonBuilder().withName("Natasha Harrower").withPhone("8019394") | ||
.withEmail("[email protected]").withModule("CS2102") | ||
.withFaculty("Computing").withVenue("underworld avenue") | ||
.withTags("classmate").withAvailabilities("FRIDAY").build(); | ||
public static final Person LEONARDO = new PersonBuilder().withName("Leonardo DiCaprio").withPhone("88472234") | ||
.withEmail("[email protected]").withModule("TS2237") | ||
.withFaculty("Arts and Social Sciences").withVenue("LT13").build(); | ||
public static final Person MICHAEL = new PersonBuilder().withName("Michael Jackson").withPhone("92347123") | ||
.withEmail("[email protected]").withModule("MUA1163") | ||
.withFaculty("Music").withVenue("YSTCM-SR9").build(); | ||
Comment on lines
+69
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good move to add our manually added testing data here, forgot about CLARA though, next time can edit! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted: #97 |
||
|
||
// Manually added - Person's details found in {@code CommandTestUtil} | ||
public static final Person AMY = new PersonBuilder().withName(VALID_NAME_AMY).withPhone(VALID_PHONE_AMY) | ||
.withEmail(VALID_EMAIL_AMY).withModule(VALID_MODULE_AMY) | ||
|
@@ -100,6 +107,7 @@ public static StaffBook getTypicalStaffBook() { | |
} | ||
|
||
public static List<Person> getTypicalPersons() { | ||
return new ArrayList<>(Arrays.asList(ALICE, BENSON, CARL, CLARA, DANIEL, ELLE, FIONA, GEORGE, KAFKA, NATASHA)); | ||
return new ArrayList<>(Arrays.asList(ALICE, BENSON, CARL, CLARA, DANIEL, ELLE, FIONA, GEORGE, KAFKA, NATASHA, | ||
LEONARDO, MICHAEL)); | ||
} | ||
Comment on lines
109
to
112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ordering here is not in alphabetical order, next time we can edit this too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted: #97 |
||
} |
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.
it would be better to arrage in alphabetic order
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.
I think better to follow person's order, for readability? Also following how edit and add did their ordering.
Update: currently changed to person's order