CSC/ECE 517 Fall 2015/oss E1557 GXM

E1557. Refactoring SignUpSheetController.rb and SignUpSheet.rb

In this project, we have unit tested various methods in SignUpSheetController.rb and SignUpSheet.rb as well as refactoring these methods. We first wrote unit tests to ensure we had adequate coverage for our methods. Then, we refactored our methods with confidence found from our unit tests.

Unit Testing

For our unit testing, we used the RSpec framework. We choose this framework since legacy tests of our methods were implemented with RSpec. We also utilized the RSpec-Mocks to test functions related to the database without having to use fixtures.

Refactoring

We performed various refactorings on our code to increase the codes readability and remove redundancy.

Extracting Variables

We extracted variable from complicated statements to increase the readability of the code. In the code base we were working with, there were a variety of lines of code that were more than 80 characters long. These lines of code were hard to read. So, we split them up into multiple lines of code by extracting statements found in function calls into their own variable.

Example:

duedate_subm = TopicDeadline.where(topic_id: topic.id, deadline_type_id: DeadlineType.find_by_name('submission').id, round: j).first

Became:

deadline_type_subm = DeadlineType.find_by_name('submission').id
duedate_subm = TopicDeadline.where(topic_id: topic.id, deadline_type_id: deadline_type_subm, round: j).first

Enforce Good Ruby Conventions

We attempted to enforce good Ruby conventions while refactoring the code. This entails doing things like correcting the formatting of the code to make it more readable. In addition to this, we converted a variety of iterators to follow good ruby conventions.

For example, sign_up_sheet.rb had the function self.add_sign_up_topic(assignment_id) which contained an iteration through a set of topics while manually maintaining an index variable. Manually maintaining the index variable can be problematic for future maintenance.

i=0
@topics.each { |topic|
  # Code not relevant to example omitted.
  i = i + 1
}

We replaced these with the Ruby each_with_index iterator. This allows us to pass the responsibility of maintaining the count of the index to the iterator.

@topics.each_with_index do |topic, i|

We replaced "=>" while passing parameter by colon (:)

So that

'due_at' => due_dates[session[:duedates][j]['id'].to_s 

became

due_at: due_dates[session[:duedates][j]['id'].to_s 

Fixing Issue 580

We were not able to recreate the problem found in issue 580. After multiple attempts of trying to recreate the bug we began to examine the source code. We believe that someone else has already fixed the bug before we started working on the project.