CSC/ECE 517 Spring 2023 - E2312 + E2313. Reimplement response.rb and responses controller.rb: Difference between revisions
(→1) |
(Initial pass at write up improvements based on review feedback) |
||
| Line 8: | Line 8: | ||
The previous version of response.rb was described as "a mess." The goal of E2312 was to reimplement it to follow better coding standards. | The previous version of response.rb was described as "a mess." The goal of E2312 was to reimplement it to follow better coding standards. | ||
* Made method names clearer, most method names were opaque | * Made method names clearer, most method names were opaque and did not properly convey their functionality | ||
[[File:add_table_rows.png]] | |||
* Moved many functionalities to mixins or helper classes to not violate Single-Responsibility | * Moved many functionalities to mixins or helper classes to not violate Single-Responsibility | ||
[[File:scores_response.png]] | |||
* Reduced usage of class methods | * Reduced usage of class methods | ||
| Line 21: | Line 23: | ||
Any object with multiple scores could take advantage of a mixin that provided methods to do calculations on those scores. | Any object with multiple scores could take advantage of a mixin that provided methods to do calculations on those scores. | ||
Functionality for calculating total, average, and maximum scores were all moved to a new Scorable mixin which can be leveraged by other models moving forward. | Functionality for calculating total, average, and maximum scores were all moved to a new Scorable mixin which can be leveraged by other models moving forward. | ||
Previously that functionality was implemented directly on the response class: | |||
[[File:aggregate_score.png]] | |||
Now it exists in a mixin, as demonstrated with this snippet: | |||
[[File:scorable_mixin.png]] | |||
=== Emails === | === Emails === | ||
The response model provided a method for creating emails. This was moved to a new MailMixin, which can be expanded to provide email functionality to more models in the future. | The response model provided a method for creating emails. This was moved to a new MailMixin, which can be expanded to provide email functionality to more models in the future. | ||
[[File:mail_mixin.png]] | |||
=== Metrics === | === Metrics === | ||
The response model provided a method for getting the volume of review comments on a response. This was moved to a ReviewCommentMixin as it is not necessarily specific to response models. | The response model provided a method for getting the volume of review comments on a response. This was moved to a ReviewCommentMixin as it is not necessarily specific to response models. | ||
[[File:review_comment.png]] | |||
== Improved Naming == | == Improved Naming == | ||
Fix instances of bad and opaque naming, like aggregate_question_score(), add_table_row(), and concatenate_all_review_comments() (which does more than the name implies) | |||
== Testing == | == Testing == | ||
Revision as of 19:59, 27 March 2023
In order to accommodate issues with another team, our team picked up another member and did both projects. The summaries of the projects are combined here to reflect that.
E2312 Reimplement Response
Summary of Changes
The previous version of response.rb was described as "a mess." The goal of E2312 was to reimplement it to follow better coding standards.
- Made method names clearer, most method names were opaque and did not properly convey their functionality
- Moved many functionalities to mixins or helper classes to not violate Single-Responsibility
- Reduced usage of class methods
Functionality Moved
This section covers the functionality that was moved out of the response model.
Score Calculation
Score calculation is not inherently tied to the idea of a response. Any object with multiple scores could take advantage of a mixin that provided methods to do calculations on those scores. Functionality for calculating total, average, and maximum scores were all moved to a new Scorable mixin which can be leveraged by other models moving forward.
Previously that functionality was implemented directly on the response class:
Now it exists in a mixin, as demonstrated with this snippet:
Emails
The response model provided a method for creating emails. This was moved to a new MailMixin, which can be expanded to provide email functionality to more models in the future.
Metrics
The response model provided a method for getting the volume of review comments on a response. This was moved to a ReviewCommentMixin as it is not necessarily specific to response models.
Improved Naming
Fix instances of bad and opaque naming, like aggregate_question_score(), add_table_row(), and concatenate_all_review_comments() (which does more than the name implies)
Testing
Many other parts of the software were stubbed out in the implementation project in order to run unit tests. The existing test suite was run against the implementation and passed successfully.
E2313 Reimplement Response Controller
Summary of Changes
The previous version of response_controller.rb was too long. It included too many functions besides the basic CRUD methods. The goal of E2313 was to reimplement it to follow better coding standards.
- Made method name to follow plural convention not singular convention.
- Moved many functionalities to mixins or helper classes
Functionality Moved
This section covers the functionality that was moved out of the responses controller.
Authentication and Authorization
There were authentication and authorization methods in the response controller. These were moved to authorization helper.
Redirect
There was a redirect function that helps redirect by params. It is moved to response helper.
Sort Reviews
There was functionality to sort reviews. We made a new function named sortReviews to perform this functionality and placed it in the response model
Singular convention to Plural convention
There were some function and classes that followed singular conventions, but were changed to follow plural convention.
Reducing code
There were multiple method invocations to set parameters (assign_action_parameters, set_content). We used the macro, before_action, so that we reduced the number of calls for these methods.
Testing
TODO