CSC/ECE 517 Spring 2021 - E2102. Refactor quiz questionnaires controller.rb

From Expertiza_Wiki
Revision as of 18:30, 18 March 2021 by Nnhimes (talk | contribs)
Jump to navigation Jump to search

Members

  • Nicholas Himes
  • Surya Makthal

Issues

  • action_allowed? needs to be refactored to use authorization utilities (see project E1915 on the Expertiza wiki)
  • Lines 23-24 contain useless comments; remove.
  • Line 26 needs a comment saying what a valid request is.
  • Line 26: if !assignment.require_quiz? ⇒ unless assignment.require_quiz?
  • Line 31: Change message to, “This assignment is not configured to use quizzes.”
  • Line 34: team_check is an unacceptable method name; the method name needs to say what the method does. Also the comment at the end of the line is useless; replace it by one that says what is going on.
  • Line 50: Why isn’t if valid.eql?("valid") simply if valid?

There also needs to be a comment saying what “valid” means in this context.

  • Comment at line 52 is confusing.
  • Line 55 needs a comment saying what it is doing.
  • Line 78: Your quiz has been taken by some other students, you cannot edit it anymore ⇒ Your quiz has been taken by other students; you cannot edit it anymore
  • create and update need to share code. Looks like create is missing some functionality present in update.
  • Line 100: What about other question types, e.g., Criterion? Or are these not permitted in quizzes? If so, what stops them from being used?

I’m not clear on what a “choice” is; please explain.

  • Line 133: Seems like this should be a method of assignment_participant; then the participant_id can be the receiver.

Suggested name: has_team_and_topic

  • Line 146: Need an explanation (comment) about what a valid question is.
  • Lines 165 ff.: Why are the update methods before the create methods?

Comment on Line 159 should say, # update checkbox question But, “question” should be replaced with “item” throughout. Why is code so different between update & create methods? Should this code be moved to the model files for the appropriate question types?

  • Line 244?: I think it would be good to flash a message before deleting a question.
  • Line 255: Not clear what a “choice type” is.
  • Lines 264 to 272: Big if-statement checks types & creates correct type of question. This should be changed to use the Factory Method pattern, shared by code for creating rubric items and code for creating quiz questions.
Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Spring_2021_-_E2102._Refactor_quiz_questionnaires_controller.rb&oldid=137946"