-
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
Add Faculty
class
#68
Changes from 12 commits
df81f7c
5b0fc0a
c10d74b
b870db1
f7c4297
c6d07f2
ef39cb0
bab057d
ae96be6
d11121f
727040a
077c6fe
d98567e
326f20f
e1b7be0
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,113 @@ | ||
package staffconnect.model.person; | ||
|
||
import static java.util.Objects.requireNonNull; | ||
import static staffconnect.commons.util.AppUtil.checkArgument; | ||
|
||
import java.util.Arrays; | ||
|
||
/** | ||
* Represents a Person's faculty in the staff book. | ||
* Guarantees: immutable; is valid as declared in | ||
* {@link #isValidFaculty(String)} | ||
*/ | ||
public class Faculty { | ||
public static final String MESSAGE_CONSTRAINTS = | ||
"The content should be a String representing a real faculty of NUS"; | ||
|
||
/** | ||
* For this version, a valid faculty value should match exactly the full name of the facult, | ||
* or the value is invalid | ||
*/ | ||
public enum FacultyEnum { | ||
ARTS_AND_SOCIAL_SCIENCES("Arts and Social Sciences"), | ||
BUSINESS("Business"), | ||
COMPUTING("Computing"), | ||
CONTINUING_AND_LIFELONG_EDUCATION("Continuing and Lifelong Education"), | ||
DENTISTRY("Dentistry"), | ||
DESIGN_AND_ENVIRONMENT("Design and Environment"), | ||
DUKE_NUS_MEDICAL_SCHOOL("Duke-NUS Medical School"), | ||
ENGINEERING("Engineering"), | ||
INTEGRATIVE_SCIENCES_AND_ENGINEERING("Integrative Sciences and Engineering"), | ||
LAW("Law"), | ||
MEDICINE("Medicine"), | ||
MUSIC("Music"), | ||
PUBLIC_HEALTH("Public Health"), | ||
PUBLIC_POLICY("Public Policy"), | ||
SCIENCE("Science"), | ||
UNIVERSITY_SCHOLARS_PROGRAMME("University Scholars Programme"), | ||
YALE_NUS_COLLEGE("Yale-NUS College"); | ||
|
||
private final String facultyName; | ||
|
||
FacultyEnum(String facultyName) { | ||
this.facultyName = facultyName; | ||
} | ||
|
||
/** | ||
* Links enum member to its name. | ||
* | ||
* @return name of the faculty | ||
*/ | ||
public String getFacultyName() { | ||
return facultyName; | ||
} | ||
} | ||
public final String value; | ||
|
||
/** | ||
* Constructs a {@code Faculty}. | ||
* | ||
* @param faculty A valid faculty. | ||
*/ | ||
public Faculty(String faculty) { | ||
requireNonNull(faculty); | ||
checkArgument(isValidFaculty(faculty), MESSAGE_CONSTRAINTS); | ||
value = fromString(faculty).getFacultyName(); // can be extended | ||
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. So far so good, though the logic is abit off currenly it is: string -> enum->string, it makes having the enum class a bit redundant. May I suggest storing the enum instead and returning the enum. This gives an additional layer of abstraction for those who use faculty. 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. I did consider that, but I wish to keep conformity as other attributes which are also strings. The fromString() function can be extended so that maybe syncronyms can be allowed. don't have to type in the full name of the faculty. Adding enum can simplify the process. |
||
} | ||
|
||
/** | ||
* Verifies if a faculty name is valid. | ||
* | ||
* @param test string representing the faculty name | ||
* @return True if the input matches any faculty name. | ||
*/ | ||
public static boolean isValidFaculty(String test) { | ||
requireNonNull(test, "faculty cannot be null"); | ||
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. Some may consider this magic string, but you don't have to fix it in this commit |
||
|
||
return Arrays.stream(FacultyEnum.values()) | ||
.anyMatch(faculty -> faculty.getFacultyName().equalsIgnoreCase(test)); | ||
} | ||
|
||
private static FacultyEnum fromString(String name) { | ||
for (FacultyEnum faculty : FacultyEnum.values()) { | ||
if (faculty.getFacultyName().equalsIgnoreCase(name)) { | ||
return faculty; | ||
} | ||
} | ||
throw new IllegalArgumentException("No enum constant matches the provided name: " + name); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return value; | ||
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. When storing value as a enum, you may choose call a toString in the enum class to return the value inside the enums instead |
||
} | ||
|
||
@Override | ||
public boolean equals(Object obj) { | ||
if (obj == this) { | ||
return true; | ||
} | ||
|
||
if (!(obj instanceof Faculty)) { | ||
return false; | ||
} | ||
|
||
Faculty otherFaculty = (Faculty) obj; | ||
return this.value.equals(otherFaculty.value); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return value.hashCode(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package staffconnect.model.person; | ||
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertFalse; | ||
import static org.junit.jupiter.api.Assertions.assertNotEquals; | ||
import static org.junit.jupiter.api.Assertions.assertTrue; | ||
import static staffconnect.testutil.Assert.assertThrows; | ||
|
||
import org.junit.jupiter.api.Test; | ||
|
||
public class FacultyTest { | ||
@Test | ||
public void constructor_null_throwsNullPointerException() { | ||
assertThrows(NullPointerException.class, () -> new Faculty(null)); | ||
} | ||
|
||
@Test | ||
public void constructor_invalidFaculty_throwsIllegalArgumentException() { | ||
String invalidFaculty = ""; | ||
assertThrows(IllegalArgumentException.class, () -> new Faculty(invalidFaculty)); | ||
} | ||
|
||
@Test | ||
public void isValidFaculty() { | ||
// null Faculty | ||
assertThrows(NullPointerException.class, () -> Faculty.isValidFaculty(null)); | ||
|
||
// invalid faculties | ||
assertFalse(Faculty.isValidFaculty("")); // empty string | ||
assertFalse(Faculty.isValidFaculty(" ")); // spaces only | ||
|
||
// valid Faculties | ||
assertTrue(Faculty.isValidFaculty("science")); // ignore capitals | ||
assertTrue(Faculty.isValidFaculty("Computing")); | ||
assertTrue(Faculty.isValidFaculty("Arts and Social Sciences")); // long faculty | ||
} | ||
|
||
@Test | ||
public void equals() { | ||
Faculty faculty = new Faculty("Computing"); | ||
|
||
// same values -> returns true | ||
assertEquals(faculty, new Faculty("Computing")); | ||
|
||
// same object -> returns true | ||
assertEquals(faculty, faculty); | ||
|
||
// null -> returns false | ||
assertNotEquals(null, faculty); | ||
|
||
// different types -> returns false | ||
assertFalse(faculty.equals(5.0f)); | ||
|
||
// different values -> returns false | ||
assertNotEquals(faculty, new Faculty("Science")); | ||
} | ||
} |
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.
For conciseness Enums don't contain the term enums in them, rather they are nouns by themselves to represent the type of items they contain. Eg: Faculties