PlainVote validation #1770
PlainVote validation #1770
Conversation
Pull reviewers statsStats of the last 30 days for popstellar:
|
|
ready for review 🎉 |
| ) { | ||
| verify() | ||
| .isNotEmptyBase64(questionId, "question ID") | ||
| .isNotEmptyBase64(electionId ?: "", "election ID") |
There was a problem hiding this comment.
I don't see any reason why we should pass an empty string when it's null rather than just pass directly electionId.
If electionId is null in the Election.generateElectionVoteId function it will throw the illegal argument exception, so we can safely check if it's null right from the beginning in the MessageValidator
There was a problem hiding this comment.
I remember I did this because IDE was screaming about it, but it's not anymore... Strange 🤷♂️
You are right tho, thanks 👍
Also is there a reason we had String? and not String for electionId? I did not want to touch it without knowing, but in my opinion we can just remove the ? to ensure we are not given a null argument.
There was a problem hiding this comment.
IIRC I left the String? instead to force String because it would have meant to change some tests that were passing null as value (they wouldn't be compiled anymore). But for the sake of the application itself, yeah it would have been better to just have String
|
|
|
|
|
addresses #1751
Adds validations et tests at construction of PlainVote.