E1920 Fix Code Climate Issues

This wiki page describes the changes are made in the Expertiza source code to implement E1920 OSS Project, "Fix Code Climate issues in models with names beginning with 'Se' thru 'Z'" in the Spring 2019 semester, CSC/ECE 517.

Introduction

This is intro section. We can talk about what is our project and how we got started.

Expertiza

Briefly talk about Expertiza.

Code Climate

Briefly talk about what is Code Climate Code Climate

Project Description

Some of the Expertiza source code violates many of the Ruby on Rails best practices and must be rectified to improve code readability, robustness and maintainability of the Expertiza application.

Our team was assigned to fix all code smells detected by Code Climate in the given set of Expertiza source code files.

Files Involved

All files in the app/models directory with names beginning with Se (e.g. section_header.rb) through Z, except users_controller.rb file were targeted by our team to be Code Climate issue free files. Therefore, our team was assigned exactly with 50 (fifty) files to fix in total. Some of the files had no issues reported by the Code Climate to begin with, and hence were not modified. However, changes/improvements performed on other files had an affect globally on the files outside of the app/models scope and also had to be modified to adapt changes we implemented. For example, renaming method is_leaf as follows:

 def leaf?

in the team_user_node.rb file require to change all callers to this method in different files accordingly. Such changes had to be done in the following files (for this particular example):

• models/assignment_node.rb
• models/node.rb
• models/questionnaire_node.rb
• views/tree_display/_row_header.html.erb

These files are outside of the requirements specifications, but we had to modify them in order to maintain correct behavior of the App. There were few such cases when we had to modify multiple files to fix one or more Code Climate issue(s). The list of all of files changed can be found in the Pull Request discussed in this article.

Ignored Code Climate Smells

There are some Code Climate issues that were ignored and not resolved intentionally.

The following issues are exempt based the given requirements specifications and were not fixed:

• Assignment Branch Condition size for [method name] is too high.
• Perceived complexity for [method name] is too high.
• Cyclomatic complexity for [method name] is too high.
• Method [method name] has a Cognitive Complexity of XX (exceeds 5 allowed). Consider refactoring.
• File [file name] has XXX lines of code (exceeds 250 allowed). Consider refactoring.
• Class [class name] has XX methods (exceeds 20 allowed). Consider refactoring.
• Method [method name] has XX lines of code (exceeds 25 allowed). Consider refactoring.

There are also two Code Climate issues that our team decided to ignore after consulting our mentor and discussing their impact on the existing code and Expertiza project. It was determined that the following Code Climate issues reported as False Positive and may not need to be resolved:

• Mass assignment is not restricted using attr_accessible.
• Potentially dangerous attribute available for mass assignment.

For further clarification with regard to ignored Code Climate issues please contact our mentor.

Project Implementation

Talk about how we fixed the most common issues reported by Code Climate.

Most Common Issues Reported by Code Climate

Description of the issues.

Fixes for Code Climate Issues

Few snippets of code with good description on how things were done.

Additional Improvements

Talk about additional improvements if there are any ( I(Nikolay) had some some I will include them here).

Remaining Issues

Talk about remaining issues and further improvements.

Further Suggestions

Talk about suggestions you have on files that you fixed if there are any? May be break down some methods into several and etc?

Pull Request

Let's add some info about pull request

Testing

Talk about Testing we performed

Rspec Testing

Talk about Rspec testing - what tests we run for what files

UI Testing

We concentrated our UI testing based on how changes to Code Climate issues affected the interface between callers and methods. Two main culprits were found. The first was the change from Time.parse to Time.zone.parse. While Time.parse returns an error for invalid or unknown times, Time.zone.parse returns nil for those types. The second culprit was changing where().first to find_by(). Using where().first return empty records if nothing is found, while find_by() returns nil is nothing is found.

The user interface testing concentrated on three type of user stories. The first set of user stories was general functionality, could users still access the basic menu items from before. The second set of user stories were those that dealt with time issues, such as sorting. The third set of user stories were those that dealt with searching.

Conclusion

Talk about conclusion.

Team Contacts and Contribution

We have 3 (three) members in our team and 1 (one) mentor was assigned to us. For any project-related questions/concerns please contact us:

• Christopher Adkins - cladkins@ncsu.edu
• George Hugh - gshugh@ncsu.edu
• Nikolay G. Titov - ngtitov@ncu.edu
• Zachariah Mathews (mentor) - zmathew@ncsu.edu

If you have questions or concerns related to a specific file that was improved/modified by our team, please contact an engineer who worked on the file based on the Assignment Grid we created to keep track of our progress and contribution. Please CC in email all team members and our mentor.

References

This section is for referencing