CSC/ECE 517 Spring 2015/oss E1501 YWS: Difference between revisions
No edit summary |
No edit summary |
||
Line 39: | Line 39: | ||
===Make assignment.rb more Ruby like=== | ===Make assignment.rb more Ruby like=== | ||
There were many instances where the coding style was not according to ruby guidelines. The code was refactored to be more ruby like. Changes such as removing the return type, writing .each instead of for loop and improved naming conventions were implemented. Some of the changes are described below: | There were many instances where the coding style was not according to ruby guidelines. The code was refactored to be more ruby like. Changes such as removing the return type, writing .each instead of for loop and improved naming conventions were implemented. Some of the changes are described below: | ||
'''Before Change''' | |||
<pre> | <pre> | ||
quiz_response_mappings.each do |qmapping| | quiz_response_mappings.each do |qmapping| | ||
Line 48: | Line 48: | ||
</pre> | </pre> | ||
'''After Change''' | |||
<pre> | <pre> | ||
quiz_responses = quiz_response_mappings.select{ |qmapping| qmapping.response }.map(&:response) | quiz_responses = quiz_response_mappings.select{ |qmapping| qmapping.response }.map(&:response) | ||
</pre> | </pre> | ||
'''Before Change''' | |||
<pre> | <pre> | ||
total = 0 | total = 0 | ||
Line 60: | Line 60: | ||
</pre> | </pre> | ||
'''After Change''' | |||
<pre> | <pre> | ||
self.questionnaires.inject(0) { |total, questionnaire| total + questionnaire.get_weighted_score(self, scores) } | self.questionnaires.inject(0) { |total, questionnaire| total + questionnaire.get_weighted_score(self, scores) } | ||
Line 69: | Line 69: | ||
The queries where changed to function more like active record query. This made the queries database independent and more user friendly. | The queries where changed to function more like active record query. This made the queries database independent and more user friendly. | ||
'''Before Change''' | |||
<pre> | <pre> | ||
DueDate.where( ['assignment_id = ? and deadline_type_id >= ?', self.id, 7]).due_at | DueDate.where( ['assignment_id = ? and deadline_type_id >= ?', self.id, 7]).due_at | ||
</pre> | </pre> | ||
'''After Change''' | |||
<pre> | <pre> | ||
DueDate.where(assignment_id: self.id, deadline_type_id: 7).due_at | DueDate.where(assignment_id: self.id, deadline_type_id: 7).due_at |
Revision as of 18:34, 23 March 2015
E1501: Refactoring Assignments.rb
This page provides information about a project which aims to refactor Assignment.rb file present in Expertiza OSS.
Introduction to Expertiza
Expertiza is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned from GitHub. This application provides an efficient way to manage assignments, grades and reviews, which makes the process easier and faster when the class strength is large.
Assignment.rb is a file which contains the assignment model and the related functionality. It also contains the score module which is refactored to make the file more clean and easy to read.
Problem Statement
Classes involved:
assignment.rb scorable.rb Gemfile schema.rb
What they do
Assignment.rb along with the controller file is responsible for creating and editing the assignments. The file also contains the functions responsible for grading.
What needs to be done:
The assignment model needed a refactoring as it contains some modules which were responsible for generating assignment scores. Moreover, the querying module was database dependent which was made database independent. There were many function names and code lines which were not following the ruby guidelines. They were changed to more ruby like coding style.
Re-factored Instances
Instance 1 : Refactoring the Assignment.rb
Creation of separate model scorable.rb
There are many methods which are not directly related to assignments. These methods are responsible for generating the scores and related functions. These methods are:
1.get_scores
2.get_max_score_possible
3.compute_reviews_hash
4.get_average_score
All these methods are taken out of assignment.rb and pushed into a new file "scorable.rb". This made score generation a separate module and the dependency of scores on assignments have been reduced significantly.
Make assignment.rb more Ruby like
There were many instances where the coding style was not according to ruby guidelines. The code was refactored to be more ruby like. Changes such as removing the return type, writing .each instead of for loop and improved naming conventions were implemented. Some of the changes are described below: Before Change
quiz_response_mappings.each do |qmapping| if qmapping.response quiz_responses << qmapping.response end end
After Change
quiz_responses = quiz_response_mappings.select{ |qmapping| qmapping.response }.map(&:response)
Before Change
total = 0 self.questionnaires.each { |questionnaire| total += questionnaire.get_weighted_score(self, scores) } total
After Change
self.questionnaires.inject(0) { |total, questionnaire| total + questionnaire.get_weighted_score(self, scores) }
Instance 2 : Addition of ActiveRecord like Query
The queries where changed to function more like active record query. This made the queries database independent and more user friendly.
Before Change
DueDate.where( ['assignment_id = ? and deadline_type_id >= ?', self.id, 7]).due_at
After Change
DueDate.where(assignment_id: self.id, deadline_type_id: 7).due_at
Refactoring view code into relevant views
There were no edit/delete links to edit/delete a topic in the instructor view. These had to be conditionally added based on the role of the currently logged in user. This was to prevent students from editing/deleting topics and to keep instructors from signing up for a topic. This was implemented as follows.
Adding Edit/Delete functionality to topics tab
The current expertiza project had two views to display the Actions column in the topics table. These were _reserve_topics.html.erb and _actions.html. The first one was displaying the signup button for the student and the latter was displaying the bookmarks and rendering the _reserve_topics.html.erb after the bookmarks. On top of this we had to add the edit/delete links for the instructor. A refactor was called for. We decided to create a _all_actions.html.erb partial which had the responsibility to display the relevant actions depending upon the role of the currently logged in user. This helped us remove the separate partial to render actions pertaining to a student, namely _reserve_topics.html.erb . Also the _actions.html.erb only took care of displaying the bookmark links rather than dealing with both bookmarks and actions.
partial_view:_all_actions,html.erb
<% if ['Instructor', 'Teaching Assistant', 'Administrator', 'Super-Administrator'].include? current_user_role?.name %> <td align="center"> <%= link_to image_tag('edit_icon.png', :border => 0, :title => 'Edit Topic', :align => 'middle'), :controller=>'sign_up_sheet', :action=> 'edit', :id => topic.id %> <%= link_to image_tag('delete_icon.png', :border => 0, :title => 'Delete Topic', :align => 'middle'), sign_up_sheet_path(topic.id), method: :delete, data: {confirm: 'Are you sure?'} %> <%elsif @show_actions %> ... #Render signup button for student here. The @show_actions will be true if this partial is navigated from the SignupSheetController which is accessible only to students.
Case 2 : Cleaning up unused code
The Refactors mentioned above enable us to delete unused partials namely:
- assignments controller partials :
- _add_signup_topics.html.erb
- _reserve_topic.html.erb
- signup_sheet_controller partials :
- _reserve_topic.html.erb
Steps to verify changes
See Also
- Expertiza on GitHub
- GitHub Project Repository Fork
- The live Expertiza website
- VCL link - This might not be available after Nov. 2014
- Expertiza project documentation wiki
- The OSS project requirements doc (Fall 2014 - Expertiza)