Skip to content
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

[PE-D][Tester D] Employee ID increments when add Command fails due to integer overflow in the Leave field #205

Closed
nus-pe-script opened this issue Oct 29, 2022 · 1 comment · Fixed by #232
Assignees
Labels
priority.Medium Important / urgent type.Bug Something isn't working
Milestone

Comments

@nus-pe-script
Copy link

image.png


Labels: type.FunctionalityBug severity.High
original: TheSoggy/ped#12

@kevinchangjk
Copy link

This is in fact a subset of a more generalisable bug.

add command increments employee ID whenever an optional field is incorrectly given.

image

From the above, we see that employeeId = new EmployeeId() is called (which increments the ID) after the mandatory fields are parsed. So an invalid department is fine, for example, but if there is an invalid phone number, then the ID is already incremented regardless.

A simple fix would be to move that line to right before the creation of the Person object near the return statement, because the error catching blocks are working fine.

@kevinchangjk kevinchangjk added priority.Medium Important / urgent type.Bug Something isn't working labels Oct 29, 2022
@kevinchangjk kevinchangjk added this to the v1.4 milestone Oct 29, 2022
@kevinchangjk kevinchangjk self-assigned this Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority.Medium Important / urgent type.Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants