CSC/ECE 517 Spring 2019 - Project E1916. Fix Code Climate issues in controllers with names beginning with A through N: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 3: Line 3:
===Description of the project===
===Description of the project===
In all files in app/controllers/ with names beginning with A through N, except assignment_controller.rb. There is some code smells detected by the code climate. These violate many of the ruby/rails best practices and needs to be rectified. Our team is to fix all code smells except
In all files in app/controllers/ with names beginning with A through N, except assignment_controller.rb. There is some code smells detected by the code climate. These violate many of the ruby/rails best practices and needs to be rectified. Our team is to fix all code smells except
*Assignment Branch Condition size for [method name] is too high
    *Assignment Branch Condition size for [method name] is too high
*Perceived complexity for [method name] is too high.
    *Perceived complexity for [method name] is too high.
*Cyclomatic 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.
    *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.
    *File [file name] has XXX  lines of code (exceeds 250 allowed). Consider refactoring.
*Class [class name] has XX methods (exceeds 20 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.
    *Method [method name] has XX lines of code (exceeds 25 allowed). Consider refactoring.
        *Mass assignment is not restricted using attr_accessible.
    *Mass assignment is not restricted using attr_accessible.
        *Potentially dangerous attribute available for mass assignment.
    *Potentially dangerous attribute available for mass assignment.


===Files modified in the project===
===Files modified in the project===

Revision as of 03:21, 25 March 2019

Description of the project

In all files in app/controllers/ with names beginning with A through N, except assignment_controller.rb. There is some code smells detected by the code climate. These violate many of the ruby/rails best practices and needs to be rectified. Our team is to 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.
   *Mass assignment is not restricted using attr_accessible.
   *Potentially dangerous attribute available for mass assignment.

Files modified in the project

  1. app/controllers/questionnaires_controller.rb
  2. app/views/questionnaires/_questionnaire.html.erb
  3. app/views/questionnaires/edit_questionnaire.html.erb (deleted)
  4. spec/controllers/questionnaires_controller_spec.rb

Issues and Improvements

The assign_instructor_id method

Problems

This method had two issues:

  1. Its name was misleading because it did not actually assign a value; it simply obtained a value from the user in the session.
  2. It was delving into implementation details of the User class.
Solution

The equivalent logic was already implemented in the User class's instructor_id method, so calls to the assign_instructor_id method were simply replaced with:

session[:user].instructor_id

A description in spec/controllers/questionnaires_controller_spec.rb was updated to reflect that assign_instructor_id was no longer in scope.

The export and import methods

Problems

These methods were actually no longer being used, because the functionality they provided was moved to the export_file and import_file routes.

Solution
  • The logic was triggered by the presence of the parameters import or export in the save_all_questions action. Code that caused these parameters to be present had been commented out of _questionnaire.html.erb and edit_questionnaire.html.erb in the questionnaire views. The latter file was no longer reachable, as there was no reference to an edit_questionnaire action in config/routes.rb or any direct references to it in the questionnaires controller itself. Thus the commented-out code was removed from _questionnaire.html.erb and the edit_questionnaire.html.erb file was deleted.
  • The logic referencing the parameters and methods was removed from the save_all_questions method.