CSC/ECE 517 Spring 2019 - E1919 CodeClimate Issues
This wiki page is for the description of changes made according to the specification of E1919, OSS project.
About Expertiza
Expertiza is a platform where assignments and related quizzes are managed. On this portal instructor upload assignments/questionnaires, create list of topics for students and edit the existing assignments. Students can form teams, work on projects and submit their assignments as URL or files. It also helps in improving the work quality by allowing students to provide anonymous reviews. It is an Open source platform which is based on Ruby with Rails.
Problem Statement
The existing code of "Expertiza" has many code smells which are not in compliance with the best practices of Ruby Rails. These smells were detected by CodeClimate.
Code climate is a command line interface for the Code Climate analysis platform. It allows you to run Code Climate engines on your local machine inside of Docker containers. It provides automated code review for test coverage and maintainability.
Objective
We were assigned the task to resolve code climate issues in models with names beginning with ‘H’ thru ‘Sc’.
As per instructions, we were to fix all the issues detected by code climate except for below. Fix all code smells except
- Assignment Branch Condition size for [method name] is too high
- Perceived complexity for [method name] is too high.
- Cyclomatic complexity for [method name] is too high.
- Method [method name] has a Cognitive Complexity of XX (exceeds 5 allowed). Consider refactoring.
- File [file name] has XXX lines of code (exceeds 250 allowed). Consider refactoring.
- Class [class name] has XX methods (exceeds 20 allowed). Consider refactoring.
- Method [method name] has XX lines of code (exceeds 25 allowed). Consider refactoring.
Add inverse_of option
File: Participant.rb belongs_to :topic, class_name: 'SignUpTopic'.
In the class 'SignUpTopic', we have has_many :teams
When you have two models in a has_many, has_one or belongs_to association, the :inverse_of option in Rails tells ActiveRecord that they're two sides of the same association. Knowing the other side of the same association Rails can optimize object loading and will reference the same object in memory, instead of loading another copy of the same record.
Add dependent property
In the file roles.rb we have has_many :users
We need to specify "dependent" option here.
With associations of Active Record , we can make transitions smooth of such operations(relationships) by telling Rails that there is a connection between the two models. It would make more sense here if we add "dependent :destroy" option for users. If a particular role is deleted, all the users under that role must also disappear.
dependent :destroy
useless assignment of a variable
In the file role.rb, we have
The variable e is not being used anywhere. Such values are assigned to "_" so that it doesnt get assigned to a variable and uses space.
rescue StandardError => _
Using snake case for method names
In file roles.rb, we have
A standard good practice is to follow snake cases for naming methods.
Use find_by instead of where.first
In file scored_question.rb, we have
Rails prefers "find_by" over "where" as "find_by" returns a single record as result, whereas "where" returns an array of record, to which we need to append "first" or "each" to get single record out of it.
answer = Answer.find_by(question_id: self.id, response_id: response_id)
And, there were many more code smells which needed a fix.
Files Modified for this requirement
- app/models/http_request.rb
- app/models/Import_error.rb
- app/models/Institution.rb
- app/models/Instructor.rb
- app/models/Invitation.rb
- app/models/Join_team_request.rb
- app/models/Language.rb
- app/models/Late_policy.rb
- app/models/Logger_message.rb
- app/models/Markup_style.rb
- app/models/Menu_item.rb
- app/models/Menu.rb
- app/models/Metareview_questionnaire.rb
- app/models/Metareview_response_map.rb
- app/models/Missing_object_id_error.rb
- app/models/Multiple_choice_checkbox.rb
- app/models/Multiple_choice_radio.rb
- app/models/Node.rb
- app/models/Notification.rb
- app/models/On_the_fly_calc.rb
- app/models/Participant.rb
- app/models/Password_reset.rb
- app/models/Path_error.rb
- app/models/Permission.rb
- . app/models/Plagiarism_checker_assignment_submission.rb
- app/models/Plagiarism_checker_comparison.rb
- app/models/question_advice.rb
- app/models/Questionnaire_header.rb
- app/models/Questionnaire_node.rb
- app/models/Questionnaire.rb
- app/models/Questionnaire_type_node.rb
- app/models/Question.rb
- app/models/Quiz_assignment.rb
- app/models/Quiz_question_choice.rb
- app/models/Quiz_questionnaire.rb
- app/models/Quiz_question.rb
- app/models/Quiz_response_map.rb
- app/models/Quiz_response.rb
- app/models/Requested_user.rb
- app/models/Response_map.rb
- app/models/Response.rb
- app/models/review_assignment.rb
- app/models/Review_comment_paste_bin.rb
- app/models/Review_grade.rb
- app/models/Review_questionnaire.rb
- app/models/Review_response_map.rb
- app/models/Role.rb
- app/models/Rscore.rb
- app/models/rubric.rb
- app/models/Scale.rb
- app/models/Scored_question.rb
- app/models/Score_view.rb
Total Modifications
42 Files changed
215 additions
163 Deletions