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

refactor methods to handle invalid transfers #933

Merged
merged 5 commits into from
Nov 1, 2023
Merged

refactor methods to handle invalid transfers #933

merged 5 commits into from
Nov 1, 2023

Conversation

ankitsmt211
Copy link
Member

@ankitsmt211 ankitsmt211 commented Oct 24, 2023

updates:

  • transfer on short messages now opens up modal menu with empty question/text field with "untitled" as title.
  • longer messages than max length are trimmed.

image

* refactor bot transfer checks
* add check for too short message transfers
resolves #926
@ankitsmt211 ankitsmt211 requested review from a team as code owners October 24, 2023 23:01
@ankitsmt211 ankitsmt211 self-assigned this Oct 24, 2023
@ankitsmt211
Copy link
Member Author

Question, if onMessageContext was a private method, would it be better to use assert instead of validation methods i did to ensure message is of required length for modal description?

@Zabuzard
Copy link
Member

Question, if onMessageContext was a private method, would it be better to use assert instead of validation methods i did to ensure message is of required length for modal description?

Id prefer to not use assert. Its disabled unless u run a specific VM argument. Prefer using ordinary exceptions and the logger with its various logging levels instead. Theres also throw new AssertionError(...) if you want a good equivalent to an assert. Note that asserts are not for user-errors but to mark bugs in the code.

@ankitsmt211
Copy link
Member Author

Id prefer to not use assert. Its disabled unless u run a specific VM argument. Prefer using ordinary exceptions and the logger with its various logging levels instead. Theres also throw new AssertionError(...) if you want a good equivalent to an assert. Note that asserts are not for user-errors but to mark bugs in the code.

True that was just curious, from what i understand assert also throws assertion error by default. But ye it needs to be enabled.

@ankitsmt211 ankitsmt211 merged commit 10f2575 into Together-Java:develop Nov 1, 2023
9 checks passed
@ankitsmt211 ankitsmt211 deleted the bug-fix-short-message-transfer branch November 30, 2023 09:39
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.

Transfer crashes on short messages
3 participants