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.

We wrote multiple controller and model tests for corresponding functions so as to tests various scenarios and get 100 % test coverage.

E.g. If you run the test in spec/controllers/sign_up_sheet_spec.rb by command "rspec spec/controllers/sign_up_sheet_spec.rb", then you can see the 100% line coverage for controller method save_topic_deadlines() in sign_up_sheet_controller.rb. For checking the coverage, open the index.html page coverage folder and go to respective controller.

Similar, run the tests in Model and Controllers folder to see 100% test coverage for following functions.

1. Create()in SignUpSheetController
2. self.add_signup_topic() in SignUpSheet Model
3. save_topic_deadlines() in SignUpSheetController
4. self.confirmTopic() in SignUpSheet

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 

Creating New Methods

We have refactored the methods which were doing a lot of things into two or more methods so as to segregate the functionality and following the basic principal that one method should do only one thing. Also, this helped in making the methods more readable and easy to understand.

E.g. In sign_up_sheet.rb Model, "def sign_up_wailisted()" is the new method extracted from original method self.confirmTopic(). There are many more such examples which are done as part of refactoring.

How to Run Tests

1. Clone the repo

git clone https://github.com/MaxSchweizer/expertiza

2. Change current directory to cloned repository

3. Run Command:

rspec spec/controllers/sign_up_sheet_spec.rb

rspec spec/models/sign_up_sheet_spec.rb

4. Check coverage:
Open overage/index.html in browser.

Check line coverage for
a. Create()in sign_up_sheet_controller.rb in Controllers
b. self.add_signup_topic() in sign_up_sheet.rb Models
c. save_topic_deadlines() in sign_up_sheet_controller.rb in Controllers
d. self.confirmTopic() in sign_up_sheet.rb in Models

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.