-
Notifications
You must be signed in to change notification settings - Fork 18
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
Initial validation highlighting #618
Conversation
… set of validation errors instead of an `isValid` boolean; fixes bug where `treatBlankAsUndeclaredWriteIn` validation failure for certain providers wouldn't fail validation.
…to `performBasicCvrSourceValidation` so user is alerted when clicking the "add" button in the CVR tab.
…ns for candidates and CVRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Could you post a screenshot or two?
} else { | ||
candidateCodeSet.add(candidate.getCode()); | ||
} | ||
} | ||
} | ||
|
||
if (candidateCodeSet.size() > 0 && candidateCodeSet.size() != candidateNameSet.size()) { | ||
isValid = false; | ||
validationErrors.add(ValidationError.CANDIDATE_DUPLICATE_CODES); | ||
Logger.severe("If candidate codes are used, a unique code is required for each candidate!"); | ||
} | ||
|
||
if (getNumDeclaredCandidates() < 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: s/< 1/== 0/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kind of in favor of keeping it this way in case some magic cosmic ray hits a computer and it results in a negative value.
# Conflicts: # src/main/java/network/brightspots/rcv/ContestConfig.java # src/main/java/network/brightspots/rcv/TabulatorSession.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great review, thank you!
ContestConfig.validate()
and associated methods to return a set of validation errors instead of anisValid
boolean.treatBlankAsUndeclaredWriteIn
validation failure for certain providers wouldn't actually fail validation.performBasicCvrSourceValidation
so user is alerted when clicking the "add" button in the CVR tab.First pass as outlined in #137 (comment).