CSC/ECE 517 Spring 2024 - E2418. Reimplement of due date.rb (Phase 2): Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
(Created page with "==E2418. Reimplement due_date.rb== This page provides a description of the Expertiza based OSS project. __TOC__ ===About Expertiza=== [http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form tea...")
 
No edit summary
Line 1: Line 1:
==E2418. Reimplement due_date.rb==
==E2418. Reimplement due_date.rb (Phase 2)==


This page provides a description of the Expertiza based OSS project.  
This page provides a description of the Expertiza based OSS project.  
Line 11: Line 11:
[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.


== Project Changes ==
==Introduction==
In this project, the following specific changes were implemented:
We're refactoring the due_dates.rb in Expertiza, enhancing its codebase to adhere to DRY and design principles, improving readability, and reducing redundancy. Our focus includes changing instance methods to class methods and further testing.


* Renamed methods and variables to accurately describe their responsibilities.
* Refactored each method to comply with DRY (Don't Repeat Yourself), SRP (Single Responsibility Principle), and other Ruby conventions.
* Overrode the comparator operator for the DueDate class and replace the `deadline_sort` class method.
* Utilized Rails built-in APIs for tasks such as validation, scoping, and database access.
* Conducted code smell checks using Code Climate


These changes aimed to enhance code quality, organization, and adherence to best practices in the Expertiza project.
==Problem Statement==
The reimplementation project entails:
#<b>Refactoring due_date.rb:</b> Enhancing code clarity, optimizing, and adding comments for unclear lines of code to improve maintainability and readability.
#<b>Removing Redundant Methods:</b> Identifying and eliminating unused methods in the controller to reduce complexity and streamline functionality.
#<b>Instance to Class Methods:</b> Changing as many instance methods to class methods as possible.
#<b>Adherence to DRY Principle:</b> Ensuring that the reimplementation follows the Don't Repeat Yourself (DRY) principle to avoid duplication and improve code efficiency.
#<b>Testing with Rspec:</b> Writing comprehensive tests using Rspec for each method to ensure functionality and integration, followed by a video demonstration showcasing the functionality of the reimplementation.


== Current Implementation of DueDate Class ==


The current implementation of the `DueDate` class can be summarized in a few key points:
==Plan for Reimplementation of DueDate==


=== Validation of Due Date Format ===
====Instance Methods to Class Methods:====
The class includes a validation method due_at_is_valid_datetime to ensure that the due date is in a valid datetime format. It checks if the due_at attribute is present and attempts to parse it using DateTime.strptime. If parsing fails and raises an ArgumentError, the method adds an error to the due_at attribute indicating that it must be a valid datetime.


=== Default Permission Handling ===
Transforming instance methods to class methods in the DueDate class requires a strategic approach, taking into account the context and usage of these methods within the broader application. This transformation ensures that methods don't rely on the state of a particular instance of the class but rather, operate at the class level, possibly requiring additional parameters to act upon data that would have been inherent to the instance. Here's a structured plan for each instance method identified in the provided code:
The class provides a default_permission method, which retrieves the default permission for a given deadline type and permission type. It accesses the DEFAULT_PERMISSION constant defined in the DeadlineRight module and returns the corresponding permission value.


=== Current Due Date Retrieval ===
#<b>1. set_flag:</b> Currently, this method updates an attribute of a specific instance. To convert it into a class method, it would need to accept an identifier for the DueDate instance (such as id) as a parameter. The class method would then locate the specific instance within the database and update its attribute. The signature would change from set_flag to something like self.set_flag(id), where id is used to find the DueDate instance and update it.
It offers functionality through the current_due_date method to retrieve the current due date from a list of due dates. This method iterates over the provided due dates and checks if any due date lies in the future compared to the current time. It returns the first future due date encountered, indicating the current due date.


=== Flag Setting ===
#<b>2. due_at_is_valid_datetime:</b> This validation method checks if the due_at attribute of an instance is a valid datetime. To change this into a class method, it would need to accept a datetime string as a parameter. However, validations in Rails models are inherently designed to be instance methods since they operate on the attributes of a specific instance before saving or updating it. Instead of transforming this into a class method, consider whether it's more appropriate to keep as an instance method or refactor the validation logic elsewhere where it can be used more generically.
It includes a set_flag method to set the flag attribute to true for a DueDate instance. This method updates the flag attribute and saves the record to the database.


=== Due Date Copying ===
3. For the #<b><=> (spaceship operator) method</b>, transforming it into a class method doesn't follow the conventional use of class methods or the purpose of the spaceship operator. The spaceship operator is used to compare two instances of the same class. Its use as a class method would be unconventional and counterintuitive since it's designed to operate on instances. If there's a compelling reason to compare data at the class level, consider implementing a separate class method that takes identifiers or attributes of two entities as parameters to perform the comparison, but this would be a deviation from the typical use of the <=> operator.
The class implements functionality to copy due dates from one assignment to another using the copy method. It retrieves due dates associated with a given assignment ID, duplicates each due date record, assigns the new parent ID, and saves the duplicated records for the new assignment.
 
=== Due Date Setting ===
Provides a set_duedate method to set due dates for assignments with specified parameters. It creates a new DueDate instance with the provided parameters, including the deadline type, parent assignment ID, round, and saves the record to the database.
 
===Sorting of Due Dates ===
Implements a method deadline_sort to sort due dates based on the due date attribute. This method sorts the provided list of due dates in ascending order of their due dates, ensuring that due dates without due_at values are placed at the end.
 
=== Assignment Round Calculation ===
Calculates the round of the response within an assignment based on due dates using the done_in_assignment_round method. It retrieves due dates associated with the given assignment ID, sorts them by due date, and iterates over them to determine the round based on the response's creation time.
 
=== Next Due Date Retrieval ===
Offers functionality through the get_next_due_date method to retrieve the next due date for an assignment or topic. Depending on whether the assignment has staggered deadlines, it queries either AssignmentDueDate or TopicDueDate records and returns the next due date after the current time.
 
The DueDate class in the Expertiza project serves as a pivotal component responsible for managing, validating, and retrieving due dates associated with assignments, encompassing functionalities such as validation of datetime formats, permission handling, and determining the current and next due dates for assignments and topics.
 
== Improvements in the New Implementation of DueDate Class ==
 
 
 
=== Renamed methods to describe their responsibility ===
* '''Problem''': Method names might not have accurately described their functionality, leading to confusion for developers.
* '''Solution''': Method renaming with descriptive names clarified their purpose, improving code readability and comprehension for developers to understand method functionalities without deep code inspection.
 
Old Code:
<pre>
def self.current_due_date(due_dates)
..
def self.teammate_review_allowed(student)
..
def self.set_duedate(duedate, deadline, assign_id, max_round)
..
def self.deadline_sort(due_dates)
</pre>
 
New Code:
<pre>
def self.current(due_dates)
..
def self.teammate_review_allowed?(student)
..
def self.set_due_date(duedate, deadline, assign_id, max_round)
..
def self.sort_deadlines(due_dates)
</pre>
 
 
 
=== Refactored each method according to DRY, SRP, and other Ruby conventions ===
* '''Problem''': Methods were potentially repetitive, non-optimized, or didn't adhere to best practices, causing inefficiencies and confusion.
* '''Solution''': Method refactoring according to DRY, SRP, and Ruby conventions optimized the code for clarity, efficiency, and maintainability, ensuring each method has a clear purpose and follows standard practices.
 
Old Code:
<pre>
def self.current(due_dates)
# Get the current due date from list of due dates
due_dates.each do |due_date|
  if due_date.due_at > Time.now
    current_due_date = due_date
    return current_due_date
  end
end
# in case current due date not found
nil
end
</pre>
 
<pre>
def self.teammate_review_allowed?(student)
# time when teammate review is allowed
due_date = current(student.assignment.due_dates)
student.assignment.find_current_stage == 'Finished' ||
  due_date &&
    (due_date.teammate_review_allowed_id == 3 ||
    due_date.teammate_review_allowed_id == 2) # late(2) or yes(3)
end
</pre>
 
<pre>
def self.copy(old_assignment_id, new_assignment_id)
    duedates = where(parent_id: old_assignment_id)
    duedates.each do |orig_due_date|
  ..
    end
end
</pre>
 
<pre>
def self.done_in_assignment_round(assignment_id, response)
..
due_dates.reject { |due_date| due_date.deadline_type_id != 1 && due_date.deadline_type_id != 2 }
..
end
</pre>
 
 
New Code:
<pre>
def self.current(due_dates)
due_dates.detect { |due_date| due_date.due_at > Time.now }
end
 
<pre>
def self.teammate_review_allowed?(student)
# time when teammate review is allowed
due_date = current(student.assignment.due_dates)
student.assignment.find_current_stage == 'Finished' ||
  (due_date && [2, 3].include?(due_date.teammate_review_allowed_id))
end
</pre>
 
<pre>
def self.copy(old_assignment_id, new_assignment_id)
    where(parent_id: old_assignment_id).each do |orig_due_date|
    ..
    end
end
</pre>
 
<pre>
def self.done_in_assignment_round(assignment_id, response)
..
due_dates.reject { |due_date| ![1, 2].include?(due_date.deadline_type_id) }
..
end
</pre>
 
 
=== Utilized Rails built-in APIs for tasks such as validation, scoping, and database access ===
 
Old Code:
<pre>
def set_flag
self.flag = true
save
end
..
</pre>
 
<pre>
def self.set_due_date(duedate, deadline, assign_id, max_round)
submit_duedate = DueDate.new(duedate)
submit_duedate.deadline_type_id = deadline
submit_duedate.parent_id = assign_id
submit_duedate.round = max_round
submit_duedate.save
end
</pre>
 
New Code:
<pre>
def set_flag
update_attribute(:flag, true)
end
..
</pre>
 
<pre>
def self.set_due_date(duedate, deadline, assign_id, max_round)
submit_duedate = DueDate.new(duedate)
submit_duedate.update(deadline_type_id: deadline, parent_id: assign_id, round: max_round)
end
</pre>
 
 
 
 
=== Overrode the comparator operator for the DueDate class and replaced the `deadline_sort` class method ===
* '''Problem''': Existing code had a dedicated method for sorting due dates, which violated Ruby conventions.
* '''Solution''': Overrode the <=> operator and defined a new function for sorting due_dates and called that instead.
 
Old Code:
<pre>
def self.sort_deadlines(due_dates)
due_dates.sort do |m1, m2|
  if m1.due_at && m2.due_at
    m1.due_at <=> m2.due_at
  elsif m1.due_at
    -1
  else
    1
  end
end
end
</pre>
 
<pre>
..
def self.done_in_assignment_round(assignment_id, response)
    ..
    sorted_deadlines = sort_deadlines(due_dates)
    ..
end
</pre>
 
New Code:
<pre>
def <=>(other)
return nil unless other.is_a?(DueDate)
 
if due_at && other.due_at
  due_at <=> other.due_at
elsif due_at
  -1
else
  1
end
end
..
</pre>
 
<pre>
def self.done_in_assignment_round(assignment_id, response)
    ..
    sorted_deadlines = due_dates.sort
    ..
end
</pre>
 
 
== Test Plan ==
 
All the previous test cases pass in the new implementation of the system.
The following outlines the NEW scenario and cases to be tested for the <=> functionality within the system:
 
=== Sorting Due Dates ===
* Scenario: Verify that due dates are sorted in the correct order.
* Edge Case: Include due dates with identical timestamps to assess the stability of the sorting algorithm.
 
<pre>
it 'sort duedate records' do
sorted_due_dates = @due_dates
expect(sorted_due_dates.each_cons(2).all? { |m1, m2| (m1.due_at <=> m2.due_at) != 1 }).to eql false
sorted_due_dates = @due_dates.sort
expect(sorted_due_dates.each_cons(2).all? { |m1, m2| (m1.due_at <=> m2.due_at) != 1 }).to eql true
end
</pre>
 
All tests passed successfully:
[[File:DueDateTest.png|1000px|thumb|none|]]


The transformation from instance methods to class methods often means changing the scope from operating on a single, specific instance to handling a broader class-level operation. This usually involves adding parameters to methods that were previously not needed because the instance itself held the context. Moreover, for some functionalities like validations or comparisons (<=> operator), the shift might not be practical or align with Rails conventions and object-oriented principles. It's crucial to assess the necessity and implications of such changes on the overall design and maintainability of the codebase.


==References==
==References==

Revision as of 23:52, 8 April 2024

E2418. Reimplement due_date.rb (Phase 2)

This page provides a description of the Expertiza based OSS project.



About Expertiza

Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

Introduction

We're refactoring the due_dates.rb in Expertiza, enhancing its codebase to adhere to DRY and design principles, improving readability, and reducing redundancy. Our focus includes changing instance methods to class methods and further testing.


Problem Statement

The reimplementation project entails:

  1. Refactoring due_date.rb: Enhancing code clarity, optimizing, and adding comments for unclear lines of code to improve maintainability and readability.
  2. Removing Redundant Methods: Identifying and eliminating unused methods in the controller to reduce complexity and streamline functionality.
  3. Instance to Class Methods: Changing as many instance methods to class methods as possible.
  4. Adherence to DRY Principle: Ensuring that the reimplementation follows the Don't Repeat Yourself (DRY) principle to avoid duplication and improve code efficiency.
  5. Testing with Rspec: Writing comprehensive tests using Rspec for each method to ensure functionality and integration, followed by a video demonstration showcasing the functionality of the reimplementation.


Plan for Reimplementation of DueDate

Instance Methods to Class Methods:

Transforming instance methods to class methods in the DueDate class requires a strategic approach, taking into account the context and usage of these methods within the broader application. This transformation ensures that methods don't rely on the state of a particular instance of the class but rather, operate at the class level, possibly requiring additional parameters to act upon data that would have been inherent to the instance. Here's a structured plan for each instance method identified in the provided code:

  1. 1. set_flag: Currently, this method updates an attribute of a specific instance. To convert it into a class method, it would need to accept an identifier for the DueDate instance (such as id) as a parameter. The class method would then locate the specific instance within the database and update its attribute. The signature would change from set_flag to something like self.set_flag(id), where id is used to find the DueDate instance and update it.
  1. 2. due_at_is_valid_datetime: This validation method checks if the due_at attribute of an instance is a valid datetime. To change this into a class method, it would need to accept a datetime string as a parameter. However, validations in Rails models are inherently designed to be instance methods since they operate on the attributes of a specific instance before saving or updating it. Instead of transforming this into a class method, consider whether it's more appropriate to keep as an instance method or refactor the validation logic elsewhere where it can be used more generically.

3. For the #<=> (spaceship operator) method, transforming it into a class method doesn't follow the conventional use of class methods or the purpose of the spaceship operator. The spaceship operator is used to compare two instances of the same class. Its use as a class method would be unconventional and counterintuitive since it's designed to operate on instances. If there's a compelling reason to compare data at the class level, consider implementing a separate class method that takes identifiers or attributes of two entities as parameters to perform the comparison, but this would be a deviation from the typical use of the <=> operator.

The transformation from instance methods to class methods often means changing the scope from operating on a single, specific instance to handling a broader class-level operation. This usually involves adding parameters to methods that were previously not needed because the instance itself held the context. Moreover, for some functionalities like validations or comparisons (<=> operator), the shift might not be practical or align with Rails conventions and object-oriented principles. It's crucial to assess the necessity and implications of such changes on the overall design and maintainability of the codebase.

References

  1. Expertiza on GitHub
  2. GitHub Project Repository Fork
  3. The live Expertiza website
  4. Expertiza project documentation wiki
  5. Rspec Documentation
  6. Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin