CSC/ECE 517 Fall 2013/oss E806 jlv: Difference between revisions
No edit summary |
No edit summary |
||
(13 intermediate revisions by 2 users not shown) | |||
Line 25: | Line 25: | ||
===OSS Project=== | ===OSS Project=== | ||
==== | ====Classes==== | ||
* response_map.rb (94 lines) | * response_map.rb (94 lines) | ||
* feedback_response_map.rb (29 lines) | * feedback_response_map.rb (29 lines) | ||
Line 35: | Line 35: | ||
* response.rb (171 lines) | * response.rb (171 lines) | ||
====What they do?==== | ====What they do?==== | ||
Responses (all kinds) are instances of filled-out rubrics. When someone responds to a rubric, a response is created. There are different kind of responses for different kinds of rubrics. A response_map keeps track of who the reviewer and who | Responses (all kinds) are instances of filled-out rubrics. When someone responds to a rubric, a response is created. There are different kind of responses for different kinds of rubrics. A response_map keeps track of who is the reviewer and who is being reviewed. ResponseMaps have a 1 to 1 correspondence with Responses. | ||
====What we did?==== | ====What we did?==== | ||
The following activities were carried out as part of the project: | The following activities were carried out as part of the project: | ||
; New data migration : Since Responses | ; New data migration : Since Responses have a 1 to 1 correspondence with ResponseMaps, we migrated the data from <code>'''response_maps'''</code> table to <code>'''responses'''</code> table. | ||
; Removing table : After the above migration was complete, we removed the <code>'''response_maps'''</code> table from the database. | ; Removing table : After the above migration was complete, we removed the <code>'''response_maps'''</code> table from the database. | ||
; Refactoring : We had to refactor the above listed classes to ensure that they continue to operate as expected after removal of the <code>'''response_maps'''</code> table. | ; Refactoring : We had to refactor the above listed classes to ensure that they continue to operate as expected after removal of the <code>'''response_maps'''</code> table. | ||
==Design | ==Design== | ||
===Database=== | ===Database=== | ||
====Existing Design==== | ====Existing Design==== | ||
[[File:E806DBDesignBefore.png]]<br/> | [[File:E806DBDesignBefore.png]]<br/><br/> | ||
In the existing design, there are two tables <code>'''responses'''</code> and <code>'''response_maps'''</code>. However, there is no need to maintain a separate table called <code>'''response_maps'''</code> since there is a 1:1 mapping between a <code>'''responses'''</code> and <code>'''response_maps'''</code> table entry. | In the existing design, there are two tables <code>'''responses'''</code> and <code>'''response_maps'''</code>. However, there is no need to maintain a separate table called <code>'''response_maps'''</code> since there is a 1:1 mapping between a <code>'''responses'''</code> and <code>'''response_maps'''</code> table entry. | ||
;<code>'''responses'''</code> : Maintains the version number, additional comment, created at, updated at and foreign key for <code>'''response_maps'''</code> table. | ;<code>'''responses'''</code> : Maintains the version number, additional comment, created at, updated at and foreign key for <code>'''response_maps'''</code> table. | ||
;<code>'''response_maps'''</code> : Maintains the mapping to reviewed object, reviewer and reviewee. It also maintains the type of the review - Feedback, Team Review, Meta Review, etc. | ;<code>'''response_maps'''</code> : Maintains the mapping to reviewed object, reviewer and reviewee. It also maintains the type of the review - Feedback, Team Review, Meta Review, etc. | ||
<br/> | |||
====New Design==== | ====New Design==== | ||
[[File:E806DBDesignAfter.png]]<br/> | [[File:E806DBDesignAfter.png]]<br/><br/> | ||
In the new design, there is only one table called <code>'''responses'''</code>. Each row in the <code>'''responses'''</code> table now contains the data from the corresponding row of the <code>'''response_maps'''</code> table.<br/> | In the new design, there is only one table called <code>'''responses'''</code>. Each row in the <code>'''responses'''</code> table now contains the data from the corresponding row of the <code>'''response_maps'''</code> table.<br/> | ||
In order to achieve this, we wrote a data migration that does the following: | In order to achieve this, we wrote a data migration that does the following: | ||
Line 58: | Line 58: | ||
# Updated these new columns in the <code>'''responses'''</code> with data from <code>'''response_maps'''</code> table by performing a join on <code>'''responses.map_id'''</code> and <code>'''response_maps.id'''</code> fields. | # Updated these new columns in the <code>'''responses'''</code> with data from <code>'''response_maps'''</code> table by performing a join on <code>'''responses.map_id'''</code> and <code>'''response_maps.id'''</code> fields. | ||
# Dropped the <code>'''response_maps'''</code> table | # Dropped the <code>'''response_maps'''</code> table | ||
<br/> | |||
===Models=== | ===Models=== | ||
====Existing Design==== | ====Existing Design==== | ||
[[File:E806DBModelsDesignBefore.png]] | [[File:E806DBModelsDesignBefore.png]]<br/><br/> | ||
In the existing design, there is a 1:1 composition relationship between <code>'''Response'''</code> and <code>'''ResponseMap'''</code> model i.e. <code>'''Response'''</code> has-a <code>'''ResponseMap'''</code>. | In the existing design, there is a 1:1 composition relationship between <code>'''Response'''</code> and <code>'''ResponseMap'''</code> model i.e. <code>'''Response'''</code> has-a <code>'''ResponseMap'''</code>. | ||
====New Design==== | ====New Design==== | ||
[[File:E806DBModelsDesignAfter.png]]<br/><br/> | [[File:E806DBModelsDesignAfter.png]]<br/><br/> | ||
In the new design, since there is only one database table representing <code>'''Response'''</code> and <code>'''ResponseMap'''</code> models - we have an inheritance relationship as | In the new design, since there is only one database table representing <code>'''Response'''</code> and <code>'''ResponseMap'''</code> models - we have an inheritance relationship as depicted above. The inheritance relationship allows us to localize the impact of the data migration. This was necessary because many models, views and controllers are using <code>'''ResponseMap'''</code> model. In this design, <code>'''ResponseMap'''</code> just forwards <code>ActiveRecord</code> calls to <code>'''Response'''</code> since columns from <code>'''response_maps'''<code> table are now in <code>'''responses'''</code> table. | ||
<br/> | |||
===Challenges=== | ===Challenges=== | ||
====Challenge 1==== | |||
'''Problem:''' ResponseMaps are created independently of Responses in the current version of the project. However, after merging these tables, we cannot really have a ResponseMap without a Response.<br/> | |||
'''Resolution:''' We fixed this in a way that controller like <code>grades_controller.rb</code> creates an entry in the <code>responses</code> table and later <code>response_controller.rb</code> (create action) can update this entry. | |||
====Challenge 2==== | |||
'''Problem:''' Existing code uses <code>response.id</code> and <code>response_map.id</code>. As per the outcome of our earlier discussions, there was a need to maintain the <code>map_id</code>. However, after aliasing <code>map_id</code> to id (in order to resolve calls from controllers), we found that some part of the code also refers to <code>response.id</code><br/> | |||
'''Resolution:''' Since the references to <code>response.id</code> are very few, 2 files to be specific. We updated these files to use a different attribute name i.e. <code>response.response_id</code> | |||
<br/><br/> | |||
==Refactoring== | |||
Here are some examples of refactoring that were performed throughout the code: | |||
===Use of find()=== | |||
We replaced instances of code that use the following version of <code>find()</code>: | |||
<pre> | |||
ResponseMap.find(:all, :conditions => ['reviewee_id = ? and reviewer_id = ?', participant.id, reviewer.id]) | |||
</pre> | |||
with the following version of <code>find()</code> | |||
<pre> | |||
ResponseMap.find_all_by_reviewee_id_and_reviewer_id(participant.id, reviewer.id) | |||
</pre> | |||
===Unused code=== | |||
In the following method, we found an unused local variable <code>assignment_id</code> that we removed. | |||
<pre> | |||
def self.delete_mappings(mappings, force=nil) | |||
failedCount = 0 | |||
mappings.each{ | |||
|mapping| | |||
assignment_id = mapping.assignment.id | |||
begin | |||
mapping.delete(force) | |||
rescue | |||
failedCount += 1 | |||
end | |||
} | |||
return failedCount | |||
end | |||
</pre> | |||
===Removing redundancy=== | |||
We removed some redundancy in several parts of the code. | |||
<br/><br/> | |||
==Testing== | ==Testing== | ||
We have created a YouTube video that can help reviewers verify that this project works as expected.<br/> | We have created a YouTube video that can help reviewers verify that this project works as expected.<br/><br/> | ||
[http://www.youtube.com/watch?v=XT2Kc7dveQ4 | [[File:OSSYoutubeDemo.png|link=http://www.youtube.com/watch?v=XT2Kc7dveQ4]]<br/> | ||
<br/> | |||
==Future work== | ==Future work== | ||
The future work for this project involves: | The future work for this project involves: | ||
# | # Renaming all sub-classes of <code>'''ResponseMap'''</code>, for example: <code>FeedbackResponseMap</code>, so that the names end in <code>'''Response'''</code>, for example: <code>FeedbackResponse</code>. | ||
# Remove <code>'''ResponseMap'''</code> model and have it's children inherit from <code>'''Response'''</code> instead. | # Remove <code>'''ResponseMap'''</code> model and have it's children inherit from <code>'''Response'''</code> instead. | ||
# Refactor all models, controllers and views that use <code>'''ResponseMap'''</code> to use <code>'''Response'''</code>. | # Refactor all models, controllers and views that use <code>'''ResponseMap'''</code> to use <code>'''Response'''</code>. |
Latest revision as of 04:34, 31 October 2013
E806 Remove ResponseMaps
The primary aim of this project was to create a data migration that merges all data from response_maps
table to responses
table.
Revision history | |||||
---|---|---|---|---|---|
Date | Changes | ||||
10/30/2013 | Updated section on testing: - Added link to YouTube demo | ||||
10/29/2013 | Updated section on design: - Added existing and new Database design - Added existing and new Models design | ||||
10/28/2013 | Initial version |
Introduction
Expertiza
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.<ref name='expertizagithub'>Kofink, A. (2013, July). expertiza @ GitHub Retrieved from https://github.com/expertiza/expertiza</ref>
OSS Project
Classes
- response_map.rb (94 lines)
- feedback_response_map.rb (29 lines)
- metareview_response_map.rb (107 lines)
- participant_review_response_map.rb (5 lines)
- review_response_map.rb (97 lines)
- team_review_response_map.rb (5 lines)
- teammate_review_response_map.rb (104 lines)
- response.rb (171 lines)
What they do?
Responses (all kinds) are instances of filled-out rubrics. When someone responds to a rubric, a response is created. There are different kind of responses for different kinds of rubrics. A response_map keeps track of who is the reviewer and who is being reviewed. ResponseMaps have a 1 to 1 correspondence with Responses.
What we did?
The following activities were carried out as part of the project:
- New data migration
- Since Responses have a 1 to 1 correspondence with ResponseMaps, we migrated the data from
response_maps
table toresponses
table. - Removing table
- After the above migration was complete, we removed the
response_maps
table from the database. - Refactoring
- We had to refactor the above listed classes to ensure that they continue to operate as expected after removal of the
response_maps
table.
Design
Database
Existing Design
In the existing design, there are two tables responses
and response_maps
. However, there is no need to maintain a separate table called response_maps
since there is a 1:1 mapping between a responses
and response_maps
table entry.
responses
- Maintains the version number, additional comment, created at, updated at and foreign key for
response_maps
table. response_maps
- Maintains the mapping to reviewed object, reviewer and reviewee. It also maintains the type of the review - Feedback, Team Review, Meta Review, etc.
New Design
In the new design, there is only one table called responses
. Each row in the responses
table now contains the data from the corresponding row of the response_maps
table.
In order to achieve this, we wrote a data migration that does the following:
- Altered
responses
table to add the missing columns fromresponse_maps
table - Updated these new columns in the
responses
with data fromresponse_maps
table by performing a join onresponses.map_id
andresponse_maps.id
fields. - Dropped the
response_maps
table
Models
Existing Design
In the existing design, there is a 1:1 composition relationship between Response
and ResponseMap
model i.e. Response
has-a ResponseMap
.
New Design
In the new design, since there is only one database table representing Response
and ResponseMap
models - we have an inheritance relationship as depicted above. The inheritance relationship allows us to localize the impact of the data migration. This was necessary because many models, views and controllers are using ResponseMap
model. In this design, ResponseMap
just forwards ActiveRecord
calls to Response
since columns from response_maps
table are now in
responses
table.
Challenges
Challenge 1
Problem: ResponseMaps are created independently of Responses in the current version of the project. However, after merging these tables, we cannot really have a ResponseMap without a Response.
Resolution: We fixed this in a way that controller like grades_controller.rb
creates an entry in the responses
table and later response_controller.rb
(create action) can update this entry.
Challenge 2
Problem: Existing code uses response.id
and response_map.id
. As per the outcome of our earlier discussions, there was a need to maintain the map_id
. However, after aliasing map_id
to id (in order to resolve calls from controllers), we found that some part of the code also refers to response.id
Resolution: Since the references to response.id
are very few, 2 files to be specific. We updated these files to use a different attribute name i.e. response.response_id
Refactoring
Here are some examples of refactoring that were performed throughout the code:
Use of find()
We replaced instances of code that use the following version of find()
:
ResponseMap.find(:all, :conditions => ['reviewee_id = ? and reviewer_id = ?', participant.id, reviewer.id])
with the following version of find()
ResponseMap.find_all_by_reviewee_id_and_reviewer_id(participant.id, reviewer.id)
Unused code
In the following method, we found an unused local variable assignment_id
that we removed.
def self.delete_mappings(mappings, force=nil)
failedCount = 0
mappings.each{
|mapping|
assignment_id = mapping.assignment.id
begin
mapping.delete(force)
rescue
failedCount += 1
end
}
return failedCount
end
Removing redundancy
We removed some redundancy in several parts of the code.
Testing
We have created a YouTube video that can help reviewers verify that this project works as expected.
Future work
The future work for this project involves:
- Renaming all sub-classes of
ResponseMap
, for example: FeedbackResponseMap
, so that the names end in Response
, for example: FeedbackResponse
.
- Remove
ResponseMap
model and have it's children inherit from Response
instead.
- Refactor all models, controllers and views that use
ResponseMap
to use Response
.
References
<references />
Notes
- All references and further reading links are in the APA style
- All the content on this page is from websites that have a Attribution-NonCommercial-NoDerivs 3.0 or similar license that allows us to use their content.