CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb
Introduction
Introduction to Expertiza
Expertiza is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using Ruby on Rails framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments. Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.
Problem Statement
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible. The solution is the reputation system. Reputation systems have been deployed as web services, peer-review researchers will be able to use them to calculate scores on assignments, both past and present (past data can be used to tune the algorithms).
For this project, our team's job is to refactor the file: reputation_web_service_controller.rb. This file is the controller to calculate the reputation scores. A “reputation” measures how close a reviewer’s scores are to other reviewers scores. This controller is the sub-class of Application_Controller, in this controller, it implements the calculation and query of reputation scores.
Issues to be fixed
Reputation_web_service_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These methods need to be broken down into simpler and more specific methods that are easier to read/understand. Also, the few instances of code duplication that exist should be removed.
- Replace class variable with a class instance var, and change all other places using these variables.
- Refactor method db_query
- Use zero? method instead of == 0
- Use find_by instead of dynamic method
- Delete lines which is wrong or useless.
- Comment lines
Modified Files
reputation_web_service_controller.rb
Approach taken to resolve the issues
Replace class variable with a class instance var, and change all other places using these variables.
In ruby, class variables are started with '@@' and this class variable are shared in the whole inherit chain, and class variables are shared by all objects of a class. For instance variables, in Ruby, instance variables are started with '@' and instance variables belong to one object, can not be used by sub-classes.
In the project, we are asked to replace the class variable with a class instance variables. So, as the figure 1 shown, in variables declaration part, we changed all class variables to instance variables.
Next, we need to change all other places using these variables.
In action send_post_request
Refactor method db_query
In the Reputation_web_service controller, the action db_query is important, because it implements the query of peer review grade.To refactor the db_query method, we need to firstly focus on the input variables of this method, there are 4 variables need inputs. The db_query method is shown as follow:
def db_query(assignment_id, another_assignment_id = 0, round_num, hasTopic)
...
end
There is a problem: Optional arguments should appear at the end of the argument list, in this method, another_assignment_id is a optional arguments, but in the original version, the optional arguments -- another_assignment_id just appear right behind the argument--assignment_id, so we need to refactor it.
And the same problems occurs when we assigns value to variable @results. We also need to change it.
There is another problem in the db_query method, we can see there is a input variable named hasTopic, but actually that's not follow the "good Ruby and Rails coding practices", so we need to refactor it which shown as follow:
Finally, The project asked us 'Move Line 26-50 before this method', it's important for code refactor, because it's very inelegant if you put the comments inside the method. If others want to read your codes, it will cost their much more time so that it needs to be changed. Simply, we can just move the whole comments before the method, and that will make the codes easily for reading.
