CSC/ECE 517 Fall 2021 - E2120. Refactor reputation web service controller.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
No edit summary
Line 37: Line 37:
   Renamed db_query to fetch_peer_reviews
   Renamed db_query to fetch_peer_reviews
db_query method name is not conveying what exactly it's doing within the method. Also, the method names should be verbs.
db_query method name is not conveying what exactly it's doing within the method. Also, the method names should be verbs.
[[File:Db_query.png|1000px]]
[[File:Db_query.png|1000px]]


   Renamed db_query_with_quiz_score to fetch_quiz_scores
   Renamed db_query_with_quiz_score to fetch_quiz_scores
db_query_with_quiz_score is not a verb. Since all method names should be verbs it is renamed to fetch_quiz_scores.
db_query_with_quiz_score is not a verb. Since all method names should be verbs it is renamed to fetch_quiz_scores.
[[File:Db_query_with_quiz_score.png|1000px]]
[[File:Db_query_with_quiz_score.png|1000px]]


   Renamed json_generator to generate_json
   Renamed json_generator to generate_json
json_generator is not a verb. Since all method names should be verbs it is renamed to generate_json.
json_generator is not a verb. Since all method names should be verbs it is renamed to generate_json.
[[File:Json_generator.png|1000px]]
[[File:Json_generator.png|1000px]]


   Renamed client to fetch_assignment_details
   Renamed client to fetch_assignment_details
The client method name seems inappropriate and doesn't convey what the method is doing.
The client method name seems inappropriate and doesn't convey what the method is doing.
[[File:Client.png|1000px]]
[[File:Client.png|1000px]]


===''' Fixed spelling of dimention'''===
===''' Fixed spelling of dimention'''===
[[File:Dimension.png|1000px]]
[[File:Dimension.png|1000px]]


===''' Removed Unnecessary code'''===
===''' Removed Unnecessary code'''===
The logic of Assignment ids 724, 735, 756 is removed from the send_post_request method is removed, since it's used to demo for paper publishing way back in 2015 and it's not being used anymore.
The logic of Assignment ids 724, 735, 756 is removed from the send_post_request method is removed, since it's used to demo for paper publishing way back in 2015 and it's not being used anymore.
[[File:Unused_code.png|1000px]]
[[File:Unused_code.png|1000px]]


Line 61: Line 67:
===''' Created new method to encrypt request body'''===
===''' Created new method to encrypt request body'''===
send_post_request method is too big and needs to be refactored in order to handle the single responsibility principle. It has the logic of encrypting the request body. So, it is moved to a separate method and named encrypt_review_data.
send_post_request method is too big and needs to be refactored in order to handle the single responsibility principle. It has the logic of encrypting the request body. So, it is moved to a separate method and named encrypt_review_data.
[[File:Encrypt.png|1000px]]
[[File:Encrypt.png|1000px]]


===''' Created new method to decrypt response body'''===
===''' Created new method to decrypt response body'''===
send_post_request method has the logic of decrypting the response body. So, it is moved to a separate method and named decrypt_review_data.
send_post_request method has the logic of decrypting the response body. So, it is moved to a separate method and named decrypt_review_data.
[[File:Decrypt.png|1000px]]
[[File:Decrypt.png|1000px]]


===''' Created new method to update reputation scores'''===
===''' Created new method to update reputation scores'''===
send_post_request method has the logic to update reputation scores. So, it is moved to a separate method and named update_reputation.
send_post_request method has the logic to update reputation scores. So, it is moved to a separate method and named update_reputation.
[[File:Update_reputation.png|1000px]]
[[File:Update_reputation.png|1000px]]



Revision as of 01:59, 19 October 2021

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 the submission of different file types for assignments, including the URLs and wiki pages.

About Reputation Web Service Controller

Expertiza allows student work to be peer-reviewed since peers can provide more feedback than the instructor can. 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).

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 (for e.g. send_post_request). 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.

  1. There is a lot of unused/commented code, which should be removed.
  2. Figure out what the code is doing and write appropriate comments for it.
  3. Rename bad method names such as (db_query, json_generator). Method names should be verbs and should say what the method does. Method names should not be so general that they could apply to many different methods.
    1. In the case of db_query, the name should say what it queries for.
      1. Also, this method not only queries, but calculates sums. Since each method should do only one thing, the code for calculating sums should be in another method.
      2. And there should be comments in the code!
    2. json_generator should be generate_json
      1. There needs to be a method comment saying what the parameters are.
  4. In send_post_request, there are references to specific assignments, such as 724, 735, and 756. They were put in to gather data for a paper published in 2015. They are no longer relevant and should be removed.
    1. send_post_request is 91 lines long, far too long.
  5. There is a password for a private key in the code (and the code is open-sourced!) It should be in the db instead.
  6. Fix spelling of “dimention”.
  7. client is a bad method name; why is stuff being copied from class variables to instance variables?

Steps taken to resolve the issues


Separated out peer grade calculation

 Separated the logic of peer grade calculation from fetch_peer_reviews method as it's doing multiple things under a single method.

fetch_peer_reviews method is not following the single responsibility principle. It's actually calculating the peer review grades and performing a db query within the same method which should not be the case.

Renaming the methods

 Renamed db_query to fetch_peer_reviews

db_query method name is not conveying what exactly it's doing within the method. Also, the method names should be verbs.

 Renamed db_query_with_quiz_score to fetch_quiz_scores

db_query_with_quiz_score is not a verb. Since all method names should be verbs it is renamed to fetch_quiz_scores.

 Renamed json_generator to generate_json

json_generator is not a verb. Since all method names should be verbs it is renamed to generate_json.

 Renamed client to fetch_assignment_details

The client method name seems inappropriate and doesn't convey what the method is doing.

Fixed spelling of dimention

Removed Unnecessary code

The logic of Assignment ids 724, 735, 756 is removed from the send_post_request method is removed, since it's used to demo for paper publishing way back in 2015 and it's not being used anymore.


Created new method to encrypt request body

send_post_request method is too big and needs to be refactored in order to handle the single responsibility principle. It has the logic of encrypting the request body. So, it is moved to a separate method and named encrypt_review_data.

Created new method to decrypt response body

send_post_request method has the logic of decrypting the response body. So, it is moved to a separate method and named decrypt_review_data.

Created new method to update reputation scores

send_post_request method has the logic to update reputation scores. So, it is moved to a separate method and named update_reputation.

Moved sensitive information to db

reputation_web_Service_controller contains private keys. Since expertiza is an open source project moved them to db.

Removed class variables

Instance variables were assigned to class variables in the controller which is a bad practice. So, class variables are removed from the controller.


Added comments in the code for better readability