|
|
| Line 25: |
Line 25: |
| ==Background== | | ==Background== |
| There were no functional tests written on the two controllers to detect bugs, as well as confirming they are functional by itself and after integration. | | There were no functional tests written on the two controllers to detect bugs, as well as confirming they are functional by itself and after integration. |
| Our goal is to add comprehensive functional tests and integration tests to the two controllers to provide a preferable test suite for the future developers.
| |
|
| |
|
| ==Steps to set up Expertiza==
| | Our goal is to add comprehensive functional tests and integration tests to the two controllers to provide a preferable test suite for the future developers. Also, as the classes are influenced by the changes from other, we are also responsible for fixing some of the bugs, refactoring the method names to a better ones, as well as reporting the bugs found in the testing process. |
|
| |
|
| * Extract source code for Expertiza into RubyMine from the following URL: https://github.com/mdjayaprakash/expertiza/tree/sign_up_sheet_controller
| |
|
| |
| * Confirm the SDK for RubyMine to Ruby1.9.3 using File -> Settings.
| |
|
| |
| * Run - bundle install: This should install all the gems required for Expertiza in Ruby1.9.3.
| |
|
| |
| * Run - rake db:create:all
| |
|
| |
| * Run - rake [http://guides.rubyonrails.org/migrations.html| db:migrate ]
| |
|
| |
| * From the Expertiza folder(i.e. project root), execute the command “rails server”
| |
|
| |
| * This should start the Expertiza server at localhost:3000 in your browser.
| |
|
| |
| ==Categories of code smells identified==
| |
|
| |
| * '''Obvious Conditionals:''' Branches that check the conditions that are pretty obvious from the context hence increase the length of the overall code and leads to inefficient code.
| |
|
| |
| * '''Meaningless Identifiers:''' The identifiers used to address some data whose meaning can’t be understood without going through the flow of code.
| |
|
| |
| * '''Excessive Parameters:''' Making a method call with too many parameters makes the code pretty much error prone and reduces the code readability, understandability and simplicity.
| |
|
| |
| * '''Lengthy Definitions:''' These are the methods that have too many responsibilities and collaborators. As a result of which, their overall length has grown too long reducing the maintainability and readability.
| |
|
| |
| * '''Duplicated Code:''' These are those pieces of code that does almost the same functionality of some other piece of code leading to a bad design style. They are usually too error prone since not everyone identifies all the code responsible for the same task and does the changes whenever the need arises.
| |
|
| |
| * '''Bad method names:''' A good method name is expected to adhere to the Ruby style and design style guidelines. It is expected to convey a reasonable amount of responsibilities handled. A good method name in combination with good practices should resemble a user story.
| |
|
| |
| * '''Used and unnecessary methods:''' These are those methods that were useful when the code was first developed. But, they lost their functionality as the code got updated and new functionalities were added or existing ones were modified.
| |
|
| |
| * '''Bad Debug Statements:''' These statements are highly useful in development environment, but they spoil the software completeness if they’re not logged in the conventional way.
| |
|
| |
|
| |
| ==Steps followed to eliminate code smells==
| |
|
| |
|
| |
| 1. '''Definition renaming:'''
| |
|
| |
| * confirmTopic() was refactored to confirm_topic()
| |
|
| |
| This method follows the [https://en.wikipedia.org/wiki/CamelCase| camel case] which is a bad method naming style was renamed to a snake case style and its functionality was moved to waitlist controller since its functionality and association with waitlist controller is more prominent.
| |
|
| |
|
| |
| [[ File:1_Screenshot.png ]]
| |
|
| |
|
| |
| * signup() was refactored to sign_up()
| |
|
| |
| Even this method follows the camel case which is a bad method naming style was renamed to a snake case style just like the previous definition. The changes were done to the method heading along with all the code associated with this method like views/sign_up_sheet/_reserve_topic.html.erb and test/functional/SignUpControllerTest
| |
|
| |
|
| |
| [[ File:2_Screenshot.png ]]
| |
|
| |
|
| |
| * teamdetails() was refactored to show_team()
| |
|
| |
| This definition does the job of fetching and showing a team. Hence, we changed its name to a word which describes the definition’s responsibility the closest.
| |
| This change had effects in views/sign_up_sheet/_table_line.html.erb
| |
|
| |
|
| |
| [[ File:3_Screenshot.png ]]
| |
|
| |
|
| |
| * signup_topics() was refactored to list()
| |
|
| |
| This definition’s responsibility can be best described by the word list since it lists all the topics for an assignment. The changes were reflected in signup_sheet_controller, views/student_task/view.html.erb, config/routes.rb and signupControllerTest in test/functional.
| |
|
| |
|
| |
| [[ File:4_Screenshot.png ]]
| |
|
| |
|
| |
| * create_default_for_microtask() was refactored to add_default_microtask()
| |
| This definition adds the default microtask as suggested by the new definition name.
| |
|
| |
|
| |
| [[ File:Screenshot 5.png ]]
| |
|
| |
|
| |
| * create_topic_deadline() was refactored to assign_topic_deadline()
| |
| The responsibility of this definition is better described as assign_topic_deadline rather than create_topic_deadline since this method does both creating and assigning task.
| |
|
| |
|
| |
| [[ File:Screenshot 6.png ]]
| |
|
| |
|
| |
|
| |
| 2. Several debug statements were present in various parts of the code which reflects bad programming style. Testing should have been used instead to identify the errors in code. For example, in sign_up:
| |
|
| |
|
| |
| [[ File:Screenshot 7.png ]]
| |
|
| |
|
| |
| 3. Several methods and parts of code that were associated with the signup_sheet_controller should be a part of other controller or model class. Such fragments of code were moved to the respective classes. The methods touched were:
| |
|
| |
|
| |
|
| |
| * The method build_dependency_graph() was moved to sign_up_topic model
| |
|
| |
| * The code in confirm_topic() was moved to waitlist_teams method in waitlist model.
| |
|
| |
| * The code in create_topic_deadline was moved to DueDate.assign_topic_deadline
| |
|
| |
| * The method set_start_due_date was moved to DueDate model
| |
|
| |
| * The functionalities of delete_signup_for_topic was moved from sign_up_sheet_controller to reassign_topic method in sign_up_topic model
| |
|
| |
| * The functionality of signup method from sign_up_sheet controller to signup_teams method in signup_sheet model.
| |
|
| |
|
| |
|
| |
| 4. Redundancy is not a desirable feature in [https://en.wikipedia.org/wiki/Object-oriented_programming| O-O style programming]. The code in “save_topic_deadlines” had some duplication in the conditional if statement. This was carefully removed to adhere to good programming style and to increase readability.
| |
|
| |
|
| |
| [[ File:Screenshot9.png ]]
| |
|
| |
|
| |
|
| |
| 5. Wherever possible, the conditional if statement was replaced by the ternary operator ( ? : ) and if !(condition) was replaced with unless to enhance readability. These changes were made in almost all the definition. Like the one showed below:
| |
|
| |
|
| |
| [[ File:Screenshot 8.png ]]
| |
|
| |
|
| |
| 6. Long definitions make it difficult to follow the flow of code and therefore, concise methods containing fewer lines of code are desired. In order to achieve the same, a new method “set_values_for_new_topic” was defined.
| |
|
| |
|
| |
| [[ File:Screenshot 10.png ]]
| |
|
| |
|
| |
|
| |
| 7. All the references to resubmission and re-review deadlines were removed and refactored. Since this functionality has already been removed, the references are redundant and useless.
| |
|
| |
|
| |
|
| |
| 8. The following redundant methods were removed since they’re redundant and were having duplicate code: find_team_members(team_id) and has_user(user, team_id)
| |
|
| |
|
| ==Testing== | | ==Testing== |
|
| |
|
| Framework Used: Capybara<ref>http://rubygems.org/gems/capybara </ref> | | Framework: Rails default testframework<ref>http://guides.rubyonrails.org/testing.html</ref> |
| Total Number of Tests: 7 | | Sample data: Fixtures |
| | Total Number of Tests: ?? |
|
| |
|
| ===Testing Scenarios:=== | | ===Testing Scenarios:=== |
| | | ====StudentTeamController==== |
| # To login successfully. | | # View student team (GET #view) |
| # To successfully login and create a private assignment with Topics and Available to Students options selected. | | # Edit team (GET #edit) |
| # To successfully login and create a Topic for the assignment created in the above test case. | | # Create team with valid name (POST #create) |
| # To login as a student who is enrolled for the same course as that of assignment created in scenario 2. | | # Create team with name in use (POST #create) |
| # To login as the same student in scenario 4 and reach the sign up sheet for the created assignment. | | # Update valid team name (POST #update) |
| # To login as a student and successfully select the created topic under the created sample test_assignment. | | # Update team name in use (POST #update) |
| # To login as a student and fail to choose another topic once a topic has already been selected. | | # Update with current team name (POST #update) |
| | | # Advertise for partners (GET #advertise_for_partners |
| | | # Remove advertise for partners (GET #remove) |
| | # Leave team |
| ==Future Works== | | ==Future Works== |
| ==References== | | ==References== |
| <references/> | | <references/> |