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

Matrix: incomplete answer list not marked as expected. #826739 #7

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

BruceGoodGuy
Copy link
Contributor

Hi, @AnupamaSarjoshi,
cc @timhunt,

I have finished development for this ticket. To reduce code duplication, I created a protected method validate_response_fields, which handles the verification logic for is_complete_response and is_gradable_response. Could you please review it?
Thanks.

@timhunt
Copy link
Member

timhunt commented Oct 2, 2024

validate_response_fields may avoid duplicating one simple for loop. But, at the cost of making the code much harder to understand. Is that really a good trade-off?

For comparison, look at qtype match. Isn't this much easier to understand? https://github.com/moodle/moodle/blob/main/question/type/match/question.php#L308 (Note, there, in is_complete_response, I would not use the $complete variable. That could be written more like is_gradable_response.)

@AnupamaSarjoshi AnupamaSarjoshi merged commit cf9677d into moodleou:main Oct 2, 2024
4 checks passed
@AnupamaSarjoshi
Copy link
Contributor

Thanks @BruceGoodGuy for working on this issue. The changes look good and have been merged :)

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