Bkasliw: Initial draft

2016-10-28T21:32:58Z

Initial draft

← Older revision	Revision as of 21:32, 28 October 2016
Line 1:	Line 1:
		= '''E1463: Refactoring TreeDisplayController''' =

		__TOC__

		=Project Links=
		[https://github.com/expertiza/expertiza Github Repo]<br>
		[https://github.com/expertiza/expertiza/pull/744 Expertize Pull Request] <br>
		[https://travis-ci.org/expertiza/expertiza/builds/171173212 Travis CI Test Cases Passes] <br>


		=Setup - How To View Our Expertiza Fork Running on AWS=
		NOTE: The scope of our project was only to refactor select code. There are no feature additions or changes, so it should work just like production Expertiza.

		1) Go to [http://ec2-54-186-80-176.us-west-2.compute.amazonaws.com:3000/ our Expertiza project running on AWS]

		2) Login as Admin<br>
		'''User:''' user2<br>
		'''Password:''' password

		3) Click on "Manage..." if you are not automatically directed to the tree display.

		=Project Description=

		For this project, our team refactored the TreeDisplayController class in the [http://expertiza.ncsu.edu/ Expertiza] OSS project. This class provides access to questionnaires, review rubrics, author feedback, courses, assignments, and course evaluations. The tree display lists all of these categories with the ability for the user to "drill down" into the subcategories or just expand/collapse as needed.

		=Refactoring TreeDisplayController Issues=

		As part of the refactoring task, we had to resolve the following issues for enhancing code readability and maintainability. The tasks included:

		#Similar code found in other locations.
		#Cyclomatic and Perceived complexity for get_children_node_ng is too high.
		#Useless assignment to variable - `childNodes`.
		#Use snake_case for variable names.
		#Prefer `each` over `for`.
		#The use of `eval` is a serious security risk.
		#Avoid the use of the case equality operator `===`.
		#Line is too long.
		#`end` at 334, 2 is not aligned with `class` at 1, 0.

		==1) Similar code found in other locations==

		In order to make the codebase more [http://www.restapitutorial.com/lessons/whatisrest.html RESTful], we have replaced all the controller action redirections with a tree_display_index_path which enormously reduces the code content and as well as provides an organized approach to action redirections.

		[[File:1_before.png]]

		Changed to ⇒

		[[File:1_after.png]]

		==2) Cyclomatic and Perceived complexity for get_children_node_ng is too high==
		By creating two separate functions child_nodes_from_params and initialize_fnode_update_children, the complexity for this function was reduced.
		The initialize_fnode_update_children function was in-turn sub-functioned into update_fnode_children which updated the children nodes.
		The new function looks like this:

		[[File:2.png]]

		==3) Useless assignment to variable - `childNodes` were removed from following two places.==

		==4) Use snake_case for variable names.==
		Variable names using camelCase were renamed to use snake_case as according to ruby coding guidelines.


		==5) Prefer `each` over `for`==

		==6)The use of `eval` is a serious security risk.==
		==7) Avoid the use of the case equality operator `===`.==

		has been changed to ⇒

		==8) Line is too long.==



		=Future Considerations=

		=Conclusion=
		In conclusion, we were able to successfully refactor the tree_display_controller and related controllers and views. We improved and expanded the use of RESTful and DRY design choices, implemented the use of routing helpers, ensured that instantiation of instance variables is minimized, and overall improved the quality and efficiency of the Expertiza codebase. We also discovered a number of potential issues and additional areas for improvement.

Bkasliw at 21:01, 28 October 2016

2016-10-28T21:01:37Z

New page

File:ScreenShotOODD.png - Revision history

Bkasliw: Initial draft

Bkasliw at 21:01, 28 October 2016