E1917 Fix Code Climate Issues

About Expertiza

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].

Code Climate Issues and Problem Statements

Codeclimate is a command line interface for the Code Climate analysis platform. It could detect code smells which violate ruby/rails best practices. The task is to fix certain code climate issues detected by Code Climate analysis platform in controllers with the name begin with P to Z. Those issues includes unsafe use of methods, inappropriate syntax, non-optimal code structure and so on. There are totally 48 different types of issues fixed in this project.

Below is a list of all issues we have fixed.

1. Unprotected mass assignment

2. Use a guard clause instead of wrapping the code inside a conditional expression

3. Do not use Time.now without zone. Use one of Time.zone.now, Time.current, Time.now.in_time_zone, Time.now.utc, Time.now.getlocal, Time.now.iso8601, Time.now.jisx0301, Time.now.rfc3339, Time.now.to_i, Time.now.to_f instead

4. Move redirect_to "/" out of the conditional.

5. Block has too many lines

6. Identical blocks of code found in 2 locations. Consider refactoring.

7. Favor unless over if for negative conditions.

8. Operator = should be surrounded by a single space.

9. Line is too long

10. Extra empty line detected at method body beginning.

11. Unnecessary spacing detected.

12. Parameters should be whitelisted for mass assignment

14. Avoid using update_attribute because it skips validations.

14. end at 44, 4 is not aligned with def at 39, 2.

15. Unsafe reflection method const_get called with parameter value

16. Avoid more than 3 levels of block nesting.

17. Use each_key instead of keys.each

18. FIXME found

19. TODO found

20. Useless assignment to variable - parent

21. Convert if nested inside else to elsif

22. Move q.save out of the conditional

23. Similar blocks of code found in 3 locations. Consider refactoring

24. User controlled method execution

25. Replace class var @@assignment_id with a class instance var.

26. Avoid comparing a variable with multiple items in a conditional, use Array#include? Instead.

27. Extra blank line detected.

28. Do not prefix writer method names with set_

29. Ternary operators must not be nested. Prefer if or else constructs instead.

30. Use empty lines between method definitions.

31. Use student_review_num.zero? instead of student_review_num == 0.

32. Use normalcase for variable numbers.

33. Use only ascii symbols in comments.

34. Possible SQL injection

35. Use find_by instead of dynamic find_by_name

36. Prefer each over for

37. Use snake_case for variable names.

39. Prefer Date or Time over DateTime.

40. Omit parentheses for ternary conditions

41. Do not place comments on the same line as the end keyword.

42. end at 135, 2 is not aligned with class at 1, 0

43. Put one space between the method name and the first argument.

44. Space missing after colon.

45. Use the new Ruby 1.9 hash syntax

46. show, edit, update, destroy are not explicitly defined on the controller.

47. Rename is_user_ta? to user_ta?

48. Avoid comma after the last item of an array

Implementation

There are many minor issues such as unnecessary spacing, change "for" to "each" for loop, extra blank line detected, omitted parentheses used and so on. We're not going to show all these minor fixing on this section but just show some important issues we fixed.

Testing

The Expertiza project provides 77 rspec tests under expertiza/spec and 8 of them are related to our controllers files. After modifying those 28 files, we want to make sure these tests could still pass. We passed all the Rspec tests except for "response_controller". However, we were not requirement to fix the issues for this controllers.

Reference

[[1]]' Code Climate General Description https://github.com/codeclimate/codeclimate/blob/master/README.md