weiennn / ped Goto Github PK

After birthday m/7 is executed, the success message is shown as Listed all employees. Should it be Listed all employees with birthdays in the specified month instead? Listed all employees may be slightly deceiving as not all the employees are listed, the Listed all employees success message should only be used for the list command

The error message thrown when an invalid view_leave command is input contains "Positive MONTHS add leaves on the specified months and negative MONTHS remove them." This should not be in the error message as you are only viewing leaves, not trying to add or remove them.

Should "Line Interface" be "Command Line Interface" instead to match the acronym CLI?

Current: When you try to add two people with the same name, it will throw an error.

Recommendation: Because people may have the same name, you can use an additional field such as phone number, address, or DOB as well to differentiate if they are the same person. For example, two persons with the same name should be able to be added provided they have different birthdays or phone numbers. Alternatively, you can consider adding a unique ID attribute instead and compare it to that.

Currently: You cannot specify how many leaves that employee takes in that month. In the case of the employee wanting to take multiple days of leave in the month, you will not be able to show it.

Recommendation: Maybe make it so u can specify how many days of leaves are taken in the month also.

On the normal screen size with zoom 100%, the examples are too small to be seen.

Recommendation: Instead of side-by-side, have one on top and one on the bottom.

Currently, if there is invalid input for list, it'll throw an error which contradicts what the UG says "list z/all will be treated the same as list since HRInsight will ignore parameters outside the command format."

Recommendation: Change the UG to say it will throw an error

The error message shown when invalid view_leave commands are input is "Month(s) cannot be empty!". However, Months is not empty, it is just invalid.

Recommendation: Change the error message into something more suitable, maybe "Invalid month detected".

Starting state:

When claim 1 $/-60 is input. This is the result:

The leaves for Alex Yeoh have disappeared. Your claim command somehow interacts with the leaves causing them to disappear whenever something has been claimed. It works for both positive and negative claims

Current: When trying to change leave but with a duplicate input, HR Insight will throw and error that says "The employee's leave(s) does not change from previous state: ", which is not entirely correct as it is a different state, just with a duplicate input.

Recommendation: Change the error message to reflect it. For example "Duplicate input detected"

Only 1 wrong month has been input but the error messages states that "some is not a number".

Recommendation: Remove the "Some is not a number" part to make it more general so it can fit any of the invalid month's error messages.

Small UI issue, the second line "Remaining claim Alex Yeoh has: $4,060" has an extra space before it.

Current: UG specifies that the Find command should work for employees whose names contain any of the given keywords. However, the Find command only works if the name matches the full keyword. For example, following the UG, this should find Alex Yeoh.

However, Alex Yeoh can't be found.

Your find command only works if the keyword matches any of the words in the name. For example find Alex, will find Alex Yeoh

Recommendation: Change the UG so that it says that it will only support full word matches