CSC/ECE 517 Fall 2019 - E1942. Refactor stage deadlines in assignment.rb

Expertiza Background

Expertiza is an assignment portal developed by faculties and students at NCSU. It provides a platform for the faculties to create assignments for students. Faculties can have assignments with staged deadlines. Expertiza has support to create teams for students, track the team members and provide review for the work done by team members. Students can also provide reviews for peer team's assignment submission. Expertiza has been developed on Ruby on Rails and is available on github.

Problem Description

Following is an OSS project which deals with refactoring of stage deadlines in assignment.rb file. An assignment can have incremental deadlines for different topics in a single assignment. This project involves refactoring the functions in assignment.rb file related to stage deadlines. There were five functions to check what kind of stage an assignment is in. However, the names of these functions were ambiguous and functionalities implemented in some of them overlapped with each other. By the end of this project we have refactored these deadline functions.

Current Scenario

The assignment.rb file has the following functions implemented :
1. current_stage_name(topic_id = nil)
2. find_current_stage(topic_id = nil)
3. get_current_stage(topic_id = nil)
4. link_for_current_stage(topic_id = nil)
5. stage_deadline(topic_id = nil)

We have found the following issues with the code with respect to Ruby conventions :

Multiple calls to DueDate.get_next_due_date()

Problem with Existing code:
In the existing code, DueDate.get_next_due_date(self.id, topic_id) has been called at numerous places. This does not follow the ruby coding standards.

Solution:
New private method next_due_date(topic_id) has been added. This function returns the next_due_date by calling the get_next_due_date method on DueDate.

File: app/models/assignment.rb
Code:

def next_due_date(topic_id = nil)
 DueDate.get_next_due_date(self.id, topic_id)
end

Following places have been refactored.

Screenshots.

Checking finished status with static string “Finished”

Problem with Existing code:
The above methods have been used at various places to check finish status of assignment by comparing with “Finished”. This does not follow ruby coding standards, as we should never compare with static things. Otherwise it becomes difficult to refactor the code later.

Solution:
Assignment is said to be finished if the next due is nil. A new private method finished? has been added. It calls the above next_due_date method and returns true if the next_due_date is nil.

File: app/models/assignment.rb Code:

def finished?( topic_id = nil )
 next_due_date(topic_id).nil?
end

The check with “Finished” has been replaces with a call to finished? . The code has been refactored at following places.

Screenshots.

Return “Unknown” when topic_id is missing

Problem with existing code:
When it is staggered deadline and the topic_id is nil, “Unknown” is returned. This does not follow the DRY Principle. Never should we compare with static strings.

Solution:
To make the code more readable and understandable, and to DRY out the code we have added a new private method topic_missing? to check if the topic is missing in case of staggered deadline.

File:app/models/assignment.rb
Code:

def topic_missing?( topic_id = nil)
 topic_id.nil? and self.staggered_deadline?
end

The code has been refactored at following places:

1. TODO

find_current_stage method removed

Problem with the existing code:
The method find_current_stage has been used locally in assignment.rb and once in students controller. However internally, this method calls the next_due_date method for the assignment which has already been separated out.

Solution:
The method find_current_stage has been removed. The places where it was called has been replaced by a call to next_due_date method.

File: app/models/assignment.rb , app/controllers/student_controller.rb

Code:

Before:
 due_date = find_current_stage(topic_id)

After:
 due_date = DueDate.get_next_due_date(self.id.topic_id)

The code has been refactored in assignments.rb and the students controller at following places:

1. TODO

link_for_current_stage method removed

Problem with existing code:
The method link_for_current_stage was called once in list.html.erb. This method checks if the current assignment has any URL specified with it and would return it if present. However, the database has no such value. As a result, the if condition where the function was called always evaluated to false.

Solution:
The function link_for_current_stage has been removed. The call to this function has also been removed and the code has been refactored accordingly.

File: app/models/assignment.rb , app/views/list.html.erb

The code in list.html.erb has been refactored as follows:

1. TODO