CSC/ECE 517 Fall 2019 - E1942. Refactor stage deadlines in assignment.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
No edit summary
Line 16: Line 16:
We have found the following issues with the code with respect to Ruby conventions :
We have found the following issues with the code with respect to Ruby conventions :


<strong>Added new function next_due_date(topic_id = nil)</strong><br>
<strong>Multiple calls to DueDate.get_next_due_date()</strong><br>
Function get_next_due_date was called on DueDate model, numerous times in the file assignment.rb. A new private function next_due_date has been added which returns the next due date of the current assignment (if any), or else return null. The function call DueDate.get_next_due_date(self.id, topic_id) was replaced by a call to private method next_due_date(topic_id). The following function has been added :
 
New code introduced is as follows.<br>
Problem with Existing code:<br>
File:app/models/assignment.rb <br>
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:<br>
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 <br>
Code:
Code:
  def next_due_date(topic_id = nil)
def next_due_date(topic_id = nil)
    DueDate.get_next_due_date(self.id, topic_id)
  DueDate.get_next_due_date(self.id, topic_id)
  end
end
 
Following places have been refactored.<br>
 
Screenshots.
 


<strong>Added new function finished?(topic_id)</strong><br>
<strong>Checking finished status with static string “Finished”</strong><br>
In many parts of the code, whether the assignment is finished or not is checked by comparing the return value with 'Finished'. A new private function has been added which returns true if the next_due_date is not nil. Otherwise, it returns false.
 
File:app/models/assignment.rb <br>
Problem with Existing code:<br>
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:<br>
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:
Code:
  def finished?( topic_id = nil )
  def finished?( topic_id = nil )
    next_due_date(topic_id).nil?
  next_due_date(topic_id).nil?
  end
end


<strong>Added new function topic_missing?(topic_id)</strong><br>
The check with “Finished” has been replaces with a call to finished? . The code has been refactored at following places.<br>
The topic_id for a staggered assignment has been checked with nil and 'Unknown' is returned. This does not follow the DRY principle. A new method to check whether the topic is missing, was added.
File:app/models/assignment.rb <br>
Code:
  def topic_missing?( topic_id = nil)
    topic_id.nil? and self.staggered_deadline?
  end


== Important Links ==
Screenshots.
1. [https://152.46.19.152:8080\ VCL Link for expertiza deployed with changes] <br>
2. [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_E1942._Refactor_stage_deadlines_in_assignment.rb\ OSS documentation] <br>

Revision as of 21:01, 6 November 2019

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.