Skip to content

issue#4: branch 'iss4_ljs'#14

Merged
ljseop1030 merged 8 commits into
mainfrom
iss4_ljs
Dec 11, 2025
Merged

issue#4: branch 'iss4_ljs'#14
ljseop1030 merged 8 commits into
mainfrom
iss4_ljs

Conversation

@ljseop1030

Copy link
Copy Markdown
Collaborator

Issue #4

Summary:
Adds post-actions:

  • edit: add/remove from list
  • next: view more options
  • quit: end process

Notes:

  • I lost my local file once while writing, so there may be missing or incomplete changes :( Please review the diff carefully.

@Kimhyewon0621 Kimhyewon0621 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YOur code looks good to me!!! However there are a littlei issue?. I want to keep UI consistency. So After our project will be ready perfectly than make an issue for that problem~

@AyeongKwon AyeongKwon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your hard work...!
LGTM!💘

if (listType.equals("1")) target = mandatory;
else if (listType.equals("2")) target = optional;
else {
System.out.println("Canceled.");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be typo.
I think this is not precious,but just want to notify you.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Thanks for notifying me I'll fix it!

@mingyeonggg mingyeonggg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!! I just leave a suggestion. Please check it~

nextInput = rv.printBatchAndGetInput(this.schedules, index, this.sc);

if (nextInput.equals("next")) {
index += 5;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this feature! 🤩

However, I just found out a critical bug:

The code increases index by 5 without checking the list size. If the user keeps typing "next" after the last page, this will definitely cause an IndexOutOfBoundsException.

Please add a validation check (e.g., if (index + 5 < schedules.size())) before increasing the index to prevent this critical crash.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😯Thanks for the critical detail! I'll fix before merging it.

@ljseop1030 ljseop1030 merged commit dec8813 into main Dec 11, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants