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

Reject multiple column letters in get_input #40

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

EricccTaiwan
Copy link
Contributor

@EricccTaiwan EricccTaiwan commented Mar 12, 2025

This update adds validation to reject inputs with multiple column letters, such as "aa", which were previously treated as out of bounds instead of being explicitly rejected.

Additionally, a comment was updated for clarity, changing "row number" to "row numbers" for consistency. #38

Before

$ ./ttt
 1 |             
 2 |             
 3 |             
 4 |             
---+-------------
      A  B  C  D
X> a5
Invalid operation: index exceeds board size
X> aa
Invalid operation: index exceeds board size
X> aa5
Invalid operation: index exceeds board size
X> d5
Invalid operation: index exceeds board size
X> dd
Invalid operation: index exceeds board size
X> ^C
$

After

$ ./ttt
 1 |             
 2 |             
 3 |             
 4 |             
---+-------------
      A  B  C  D
X> a5
Invalid operation: index exceeds board size
X> aa
Invalid operation: multiple column alphabets detected
X> aa5
Invalid operation: multiple column alphabets detected
X> d5
Invalid operation: index exceeds board size
X> dd
Invalid operation: multiple column alphabets detected
X> ^C
$

This update adds validation to reject inputs with multiple column
letters, such as "aa", which were previously treated as out of
bounds instead of being explicitly rejected.

Additionally, a comment was updated for clarity, changing "row
number" to "row numbers" for consistency.

Co-authored-by: charliechiou <[email protected]>
@jserv jserv merged commit 3007f7d into jserv:main Mar 12, 2025
2 checks passed
@jserv
Copy link
Owner

jserv commented Mar 12, 2025

Thank @EricccTaiwan for contributing!

@EricccTaiwan EricccTaiwan deleted the muliple-col branch March 12, 2025 15:32
@@ -59,6 +59,14 @@ static int get_input(char player)
parseX = 1;
for (int i = 0; i < (r - 1); i++) {
if (isalpha(line[i]) && parseX) {
// input has multiple column alphabets
if (i > 0 && isalpha(line[i])) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm quite late to respond. But the isalpha(line[i]) check seems to be checked twice in the same path, so it looks like a redundant condition to me?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm quite late to respond. But the isalpha(line[i]) check seems to be checked twice in the same path, so it looks like a redundant condition to me?

Inputs like aa previously showed "index exceeds board size," which wasn’t accurate since the issue was multiple column letters. Also, isalpha(line[i]) is checked twice, which might be redundant, there could be a better way to handle this. I'll look into it!

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.

3 participants