CSC/ECE 517 Spring 2019 E1910 Refactor assignments controller.rb: Difference between revisions
Line 43: | Line 43: | ||
- edit_params_setting ** this is causing a test to fail we are currently trying to figure out why | - edit_params_setting ** this is causing a test to fail we are currently trying to figure out why | ||
== Code Improvements == | |||
The DRY principle states, "Every piece of knowledge or logic must have a single, unambiguous representation within a system."[2] This project was about furthering this principle and keeping this controller concise and maintainable. | The DRY principle states, "Every piece of knowledge or logic must have a single, unambiguous representation within a system."[2] This project was about furthering this principle and keeping this controller concise and maintainable. | ||
This is a controller that helps instructors, TA's, administrators, and super-administrators create/update/edit/show/delete current and past assignments. We updated the | This is a controller that helps instructors, TA's, administrators, and super-administrators create/update/edit/show/delete current and past assignments. We updated the aforementioned method names so that the naming convention follows the DRY principle. Many of the method names were far too long or repetitive or unhelpful in understanding what the method actually completed. In accordance with the principle we updated the method names to be simple while also being useful in letting the programmer know what the method does. | ||
Many of these methods did not have any comments to help the programmer understand the logic driving the method. To further help the programmer in understanding what the code does we added block comments at the beginning of each method that we updated. | Many of these methods did not have any comments to help the programmer understand the logic driving the method. To further help the programmer in understanding what the code does we added block comments at the beginning of each method that we updated. | ||
We cleaned up the controller by moving some methods from the controller into the associated helper file (assignment_helper.rb). By doing this we made the controller less messy and easier to read while maintaining and furthering the DRY principle. The code is now easily maintainable and all "helper" methods are in one place rather than separated across many files. | We cleaned up the controller by moving some methods from the controller into the associated helper file (assignment_helper.rb). By doing this we made the controller-less messy and easier to read while maintaining and furthering the DRY principle. The code is now easily maintainable and all "helper" methods are in one place rather than separated across many files. | ||
The create method was refactored so that the branch condition would be lower. We took a part of the functionality and put it in its own method called "update_assignment_form". This method handles updating the id of the assignment_questionnaire and the due_date and updating the form accordingly. As a result of this change two code climate issues have been resolved: | The create method was refactored so that the branch condition would be lower. We took a part of the functionality and put it in its own method called "update_assignment_form". This method handles updating the id of the assignment_questionnaire and the due_date and updating the form accordingly. As a result of this change two code climate issues have been resolved: | ||
- The length of the create method was too long | - The length of the create method was too long | ||
- The cognitive complexity has been reduced | - The cognitive complexity has been reduced | ||
The "update_assignment_form" method had redundancy of code and so we created two different functions for that part and created a generalized function to run the logic based on the option that is passed to the function. This way we overcame the redundancy part in the function by modularizing the logic. | |||
Old create method: | Old create method: |
Revision as of 22:23, 25 March 2019
This wiki page is for the description of changes made under E1910 OSS assignment for Spring 2019, CSC/ECE 517.
We have not deployed this project since we did not change or add any functionality that would affect the user.
Expertiza Background
Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages. [1]
Description of the current project
The assignments_controller has multiple functions with wrong naming convention, they are classic examples of bad function names. Few of the variables names used in the method can also be refactored to make it more relevant and understandable. In addition, functions code can be optimized to ensure that it follows 'Don't Repeat Yourself' (DRY) principle. Also, most of the functions have missing code comments, which should be added to the functions.
Files modified in current project
The only controller modified for this project was the:
1. AssignmentsController
2. Assignment_helper
The updated methods in AssignmentController:
- handle_assignment_directory_path_nonexist_case_and_answer_tagging -- missing_submission_directory - assignment_form_key_nonexist_case_handler -- assignment_submission_handler - Handle_current_user_timezonepref_nil -- timezone_handler - update_nil_dd_deadline_name -- update_due_date_deadline_name - update_nil_dd_description_url -- update_due_date_description_url - assignment_form_assignment_staggered_deadline? -- assignment_staggered_deadline? - check_due_date_nameurl_not_empty -- check_due_date_nameurl - update_feedback_assignment_form_attributes -- update_feedback_attributes
We have left a few comments in the controller that say 'used to be...' so that it can be seen where the changes have been made and these will be removed before the final submission.
The assignment_helper file was added to. Some of the methods in the controller were misplaced and according to the DRY principle these methods are better suited to be in the helper method. The methods that were moved from the controller to the helper were:
- assignment_submission_handler -> updated method name - check_due_date_nameurl(dd) -> updated method name - edit_params_setting ** this is causing a test to fail we are currently trying to figure out why
Code Improvements
The DRY principle states, "Every piece of knowledge or logic must have a single, unambiguous representation within a system."[2] This project was about furthering this principle and keeping this controller concise and maintainable.
This is a controller that helps instructors, TA's, administrators, and super-administrators create/update/edit/show/delete current and past assignments. We updated the aforementioned method names so that the naming convention follows the DRY principle. Many of the method names were far too long or repetitive or unhelpful in understanding what the method actually completed. In accordance with the principle we updated the method names to be simple while also being useful in letting the programmer know what the method does.
Many of these methods did not have any comments to help the programmer understand the logic driving the method. To further help the programmer in understanding what the code does we added block comments at the beginning of each method that we updated.
We cleaned up the controller by moving some methods from the controller into the associated helper file (assignment_helper.rb). By doing this we made the controller-less messy and easier to read while maintaining and furthering the DRY principle. The code is now easily maintainable and all "helper" methods are in one place rather than separated across many files.
The create method was refactored so that the branch condition would be lower. We took a part of the functionality and put it in its own method called "update_assignment_form". This method handles updating the id of the assignment_questionnaire and the due_date and updating the form accordingly. As a result of this change two code climate issues have been resolved:
- The length of the create method was too long - The cognitive complexity has been reduced
The "update_assignment_form" method had redundancy of code and so we created two different functions for that part and created a generalized function to run the logic based on the option that is passed to the function. This way we overcame the redundancy part in the function by modularizing the logic.
Old create method:
New create method:
update_assignment_form method:
There were many existing test cases for this controller. We have made sure that no test cases were violated by our changes. No changes were made to the associated views or model files.
Testing Details
RSpec
There were already quite a few test cases for the AssignmentsController. We have not added any new tests but made sure that no tests were failing due to our changes. No changes were made to the associated views and model file.
References
1. http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2015/oss_E1553_AAJ
2. https://dzone.com/articles/software-design-principles-dry-and-kiss