CSC/ECE 517 Spring 2020 Refactor assignment.rb

From Expertiza_Wiki
Jump to navigation Jump to search

Abstract

Expertiza is a web application using Ruby on Rails framework. The creation and maintenance of this application is handled by NCSU students and faculty. The basic functionalities allow the instructor to create and edit, both new and existing assignments. Also it allows the publishing of surveys and peer reviews, while allowing students the opportunity to sign up for topics, teams, submit assignments and peer reviews.

Problem Statement

Assignment.rb is a large file, that has dozens of methods and fields. Some methods seem redundant, and some fields and string literals are repeated multiple times throughout the file. Ie. 'Finished'. Each method in the file is essential and provides different data, so the goal for my team was to rename and merge code where we could, while not affecting the end functionality.


Problem Solution

Due to the fragile nature of the methods in Assignment.rb, our team decided to focus on redundancy in the code and anywhere we could merge redundant code into common methods. Also there were quite a few strings that were used constantly in the file, ie. 'Finished' that we decided to make constants, to reduce complexity if the values ever changed.

Refactors

One of the main issues I saw in the code was the reuse of string literals in comparisons. These would cause hige efforts to change later if there are hundreds of places where these strings are used. Instead I replaced all instances of these strings in the file with constants, to lower complexity.

Added these constants to file:

 # Constants for common fields, to reduce complexity if change is needed
 FINISHED_CONST = 'Finished'.freeze
 UNKNOWN_CONST = 'Unknown'.freeze

Removed references to string literals and made constant values

 due_date == FINISHED_CONST
 return (topic_id.nil? ? UNKNOWN_CONST : get_current_stage(topic_id))


The next issue I found was in multiple places where the code was making the same boolean checks. Instead rewtiting tge checks each time I made a method to return True/False for the check.

# Function to check if topic id is null when its a staggered assignment, to prevent redundancy
 def topic_missing?(topic_id = nil)
   topic_id.nil? and self.staggered_deadline?
 end

Testing