CSC/ECE 517 Fall 2015/oss E1551 RGS: Difference between revisions
Jump to navigation
Jump to search
Line 12: | Line 12: | ||
:<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>. | :<code>get_scores</code> has a Java-like name. It should just be <code>scores</code>. | ||
=== Delete rereview method === | === Delete rereview method === | ||
The 100+-line method <code>rearview</code> does not seem to be used anymore. The second-round review can be invoked in the same way as the first-round review. Remove it. | ;Problem definition | ||
:The 100+-line method <code>rearview</code> does not seem to be used anymore. The second-round review can be invoked in the same way as the first-round review. Remove it. | |||
;Solution summary | |||
: The method <code>rearview</code> was deleted. | |||
=== DRY create and update === | === DRY create and update === | ||
<code>create</code> and <code>update</code> use completely different code. Factor the common parts out into a partial, thereby simplifying the methods. | ;Problem definition | ||
:<code>create</code> and <code>update</code> use completely different code. Factor the common parts out into a partial, thereby simplifying the methods. | |||
;Solution summary | |||
:There was not much actual overlap in these two methods. Minimal changes. | |||
=== Consistent authorization === | === Consistent authorization === | ||
Authorization needs to be checked in the <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file. After you move the functionality, check to make sure that no one other than the author of a review (or another team member, in the case of author feedback) can edit it, then remove the <code>redirect_when_disallowed</code> method. | ;Problem definition | ||
:Authorization needs to be checked in the <code>action_allowed</code> method instead of in <code>redirect_when_disallowed</code> at the bottom of the file. After you move the functionality, check to make sure that no one other than the author of a review (or another team member, in the case of author feedback) can edit it, then remove the <code>redirect_when_disallowed</code> method. | |||
;Solution summary | |||
: | |||
=== Refactor get_content === | === Refactor get_content === | ||
<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does. | ;Problem definition | ||
:<code>get_content</code> is a complex method. It should be renamed to <code>content</code> and simplified. Comments should be added explaining what it does. | |||
;Solution summary | |||
:The <code>get_content</code> method was renamed <code>set_content</code>, simplified, and documented with comments. | |||
=== Remove SQL queries === | === Remove SQL queries === | ||
This class contains SQL queries. Please change them to Active Record commands. | ;Problem definition | ||
:This class contains SQL queries. Please change them to Active Record commands. | |||
;Solution summary | |||
:No SQL queries were identified. No changes. | |||
== Tests == | == Tests == |
Revision as of 20:11, 30 October 2015
E1551. Refactoring response_controller.rb
Expertiza
ResponseController
The ResponseController manages Responses.
Changes
Refactor latestResponseVersion method
- Problem definition
latestResponseVersion
is misnamed. It returns all versions of a response, and anyway, the method name should use underscores, not camelcase.
Rename get_scores
- Problem definition
get_scores
has a Java-like name. It should just bescores
.
Delete rereview method
- Problem definition
- The 100+-line method
rearview
does not seem to be used anymore. The second-round review can be invoked in the same way as the first-round review. Remove it. - Solution summary
- The method
rearview
was deleted.
DRY create and update
- Problem definition
create
andupdate
use completely different code. Factor the common parts out into a partial, thereby simplifying the methods.- Solution summary
- There was not much actual overlap in these two methods. Minimal changes.
Consistent authorization
- Problem definition
- Authorization needs to be checked in the
action_allowed
method instead of inredirect_when_disallowed
at the bottom of the file. After you move the functionality, check to make sure that no one other than the author of a review (or another team member, in the case of author feedback) can edit it, then remove theredirect_when_disallowed
method. - Solution summary
Refactor get_content
- Problem definition
get_content
is a complex method. It should be renamed tocontent
and simplified. Comments should be added explaining what it does.- Solution summary
- The
get_content
method was renamedset_content
, simplified, and documented with comments.
Remove SQL queries
- Problem definition
- This class contains SQL queries. Please change them to Active Record commands.
- Solution summary
- No SQL queries were identified. No changes.
Tests
Contact: Ed Gehringer, efg@ncsu.edu What it does: response_controller.rb manages Responses. A Response is what is generated any time a user fills out a rubric--any rubric. This includes review rubrics, author-feedback rubrics, teammate-review rubrics, quizzes, and surveys. Responses come in versions. Any time an author revises work, and the reviewer comes back to review it, a new Response object is generated. This Response object points to a particular ReponseMap, which tells who the reviewer is, which team is the reviewee, and what the reviewed entity is. What needs to be done:
Please write some functional tests for this class.