Refractor refresh #218
Refractor refresh #218
Conversation
| @@ -91,6 +98,16 @@ public void setSudoHrFilePath(Path sudoHrFilePath) { | |||
| userPrefs.setSudoHrFilePath(sudoHrFilePath); | |||
| } | |||
|
|
|||
| @Override | |||
| public void refresh() { | |||
There was a problem hiding this comment.
Just double confirm, setting predicate here to be true / false of an already filtered list will not introduce new unnecessary behaviour right? it wont remove all entires or display all entries right
| @@ -190,8 +190,6 @@ public void addEmployee(Employee employee) { | |||
| public void setEmployee(Employee target, Employee editedEmployee) { | |||
| requireNonNull(editedEmployee); | |||
| employees.setEmployee(target, editedEmployee); | |||
| refreshDepartments(); | |||
There was a problem hiding this comment.
defensive prog aside, just wondering where's the logic for edit employee command? Where is the new refresh method called for edit employee command?
saw it removed here but no refresh() call to EditCommand? Or is it handled elsewhere
There was a problem hiding this comment.
There should be no need to add in logic for edit employee since we are currently just tracking the number of employees in each leaves/department, editing employee has no effect
Though for defensive programming sake, it would be better to refresh it
There was a problem hiding this comment.
oh right. Normal edits to employee (eg name) will be updated because rendering logic handled diffly.
Refactored refresh to be at ModelManager Level instead of SudoHR level.
Used predicate instead of delete/addition of employees to trigger view update