Add Faculty class
#68
Conversation
* 'master' of https://github.com/JerryWang0000/tp: Update file names to follow format Update aboutus page with my details and photo Update typos Change use case for filter and typo in sort aboutus update file name changed names to match github names update aboutus info to name match lowercase requirements Updated use cases Update AboutUs for Jerry Wang
* 'master' of https://github.com/JerryWang0000/tp: Update files with traces of the package name "seedu.address" to "staffconnect" Update remaining files with traces of seedu.address in test Refactor package seedu.address to staffconnect in test Refactor package seedu.address to staffconnect in main Update header in _base.scss Fix typos and added clarity Update UserGuide description Update website main index Add pluiexo.md page Amend Readme Update github info Update Readme to fit StaffConnect Update DeveloperGuide.md Update AboutUs: Replace placeholder images Update UserGuide.md
Add Faculty and its test class.
JerryWang0000
added
type.Task
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #68 +/- ##
============================================
+ Coverage 75.36% 77.81% +2.44%
- Complexity 424 495 +71
============================================
Files 72 80 +8
Lines 1352 1537 +185
Branches 127 148 +21
============================================
+ Hits 1019 1196 +177
- Misses 303 310 +7
- Partials 30 31 +1 ☔ View full report in Codecov by Sentry. |
Restrict the valid input values to currently existing NUS faculties. Change the test class.
|
Now I store the faculty value as an enum. Do I need to change it to String? |
Change the type of value to String.
| 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.
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.
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.
| public String toString() { | ||
| return value; |
There was a problem hiding this comment.
When storing value as a enum, you may choose call a toString in the enum class to return the value inside the enums instead
| * For this version, a valid faculty value should match exactly the full name of the facult, | ||
| * or the value is invalid | ||
| */ | ||
| public enum FacultyEnum { |
There was a problem hiding this comment.
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
Rename the enum to FacultyName
| * @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.
Some may consider this magic string, but you don't have to fix it in this commit
Add Faculty and its test class.
closes #45