CSC/ECE 517 Fall 2015/oss E1552 NRR

E1552 Refactoring dynamic_review_assignment_helper.rb and dynamic_quiz_assignment_helper.rb

Overview

Refactoring

Refactoring is the process of changing a software system in such a way that it does not alter the external behavior of the code yet improves its internal structure<ref>[1]</ref>. It is a disciplined way to clean up code that minimizes the chances of introducing bugs. In essence when you refactor you are improving the design of the code after it has been written. There are many benefits of refactoring as follows: <ref>[2]</ref>

• Refactoring improves the design of software. Refactoring often cleans up codes by deleting duplicates, divides a big chunk of codes into several methods, and makes the program more understandable.
• Because refactoring makes a design cleaner, it helps the programmers understand codes better and see things that may have not been seen before.
• Refactoring helps spot bugs since it makes the software more comprehensible.
• Refactoring turns an adverse design into a good design, which allows for rapid software development.

Peer Reviewing

It is the process of evaluating work done by an individual or a team by another team or an individual with expertise in the concerned area. Types of Reviewing Strategies in Expertiza -

• Instructor Selected Reviewing - Using this strategy the instructor assigns reviews to participant in the assignment. After selecting this strategy the instructor has the following two options -
  • Set number of reviews done by each student.
  • Set minimum number of reviews done for each submission.

Then the instructor can click on assign reviewers and map the students to the topics which should be reviewed by them.

• Auto Selected Reviewing - If the instructor selects this strategy, students need to select reviews themselves. Either the student is given a list of topics to choose from and he selects which topic he wants to review, or he checks “I don’t care which topic I review” and he/she will be randomly assigned a topic to review. The instructor can also specify two parameters after selecting this strategy -
  • Review topic threshold: - Let us assume Topic 1 has a submission than has not been reviewed. Topic 2 has submissions that has been reviewed atleast 2 times and k is set 2. Then the student will not be able to select topic 2 until all of topic 1’s submission has been reviewed atleast 2 times.
  • Maximum number of reviews per submission: - This specifies the maximum number of times a particular top can be reviewed.

• Student Selected Reviewing -

In this strategy the student selects a particular submission to review instead of a particular topic. This feature had never been used and did not work correctly. Hence it has been removed from the system. However there are a lot of places where the code for student selected reviewing exists however it not being called anywhere.

Problem Statement

Classes involved:

dynamic_quiz_assignment_helper.rb
dynamic_review_assignment_helper.rb
review_mapping_controller.rb

What it does:

dynamic_review_assignment_helper.rb allows the system choose a team to review when a student chooses a topic to review. It helps the code choose topics that have submissions with fewer reviews than other topics. A threshold k is used to decide whether a particular topic can be chosen. If, e.g., Topic 1 has a submission that has not yet been reviewed, and Topic 2 has no submission that has been reviewed by fewer than 3 reviewers, then the incoming reviewer can only choose Topic 2 if the k ≥ 3. This class does not actually decide what is reviewable, but it does make up a sorted list of the submissions by the number of the reviews they have. dynamic_quiz_assignment_helper.rb is similar, but for quizzes.

What needs to be done:

• There is code for “student-selected” reviewing. This would have allowed a student to choose a particular submission, not just a particular topic. This feature had never been used, and did not work correctly, so we removed it from the system. Thus, the code for student-selected reviewing should be removed.
• The find_submissions_in_current_cycle method returns a list of participants. It should return a list of teams. Returning a list of participants leads to redundant entries in the hash, if any are done by multi-member teams. There used to be a need for dealing with participants rather than teams, but in the current Expertiza, submissions always belong to teams rather than individuals. (This may also require changing the call site for some calls to methods of this class.)
• The comment at the beginning of the class lists a number of criteria that have to hold for a submission to be reviewable (e.g., it was not written by the potential reviewer, it has not already been reviewed by the potential reviewer). It’s not clear where these conditions are checked for. I think the different conditions are checked for in different places. Sometimes they are checked for late (e.g., if a user has written on Topic 2, (s)he is allowed to select Topic 2 as a topic to review, but after (s)he selects it, (s)he is told that there are no more submissions to review on that topic). It would be better to check for all of these conditions in dynamic_review_assignment_helper; then the user would not even be able to select topics that have no available reviews.
• The two classes have a lot of duplicated code. Remove all DRY problems.

References

<references/>