CSC/ECE 517 Fall 2014/oss E1463 vpd: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
No edit summary
Line 32: Line 32:
All our code changes can be checked out at our [https://github.com/Druotic/expertiza/pull/1/files Git project repository].  
All our code changes can be checked out at our [https://github.com/Druotic/expertiza/pull/1/files Git project repository].  


combined all of the goto methods
==Combined All of the Goto Methods==
We also combined all of the goto methods into one, in accordance with the [http://en.wikipedia.org/wiki/Don't_repeat_yourself DRY] principle. This removed a significant amount of duplicate code.
 
3 of the 10 unnecessary methods removed from tree_display_controller.rb
<pre>
- # direct access to questionnaires
- def goto_questionnaires
- node_object = TreeFolder.find_by_name('Questionnaires')
- session[:root] = FolderNode.find_by_node_object_id(node_object.id).id
- redirect_to :controller => 'tree_display', :action => 'list'
- end
-
- # direct access to review rubrics
- def goto_review_rubrics
- node_object = TreeFolder.find_by_name('Review')
- session[:root] = FolderNode.find_by_node_object_id(node_object.id).id
- redirect_to :controller => 'tree_display', :action => 'list'
- end
-
- # direct access to metareview rubrics
- def goto_metareview_rubrics
- node_object = TreeFolder.find_by_name('Metareview')
- session[:root] = FolderNode.find_by_node_object_id(node_object.id).id
- redirect_to :controller => 'tree_display', :action => 'list'
- end
</pre>
 
These methods were replaced by a variable containing a list mapping of all the session variable strings to database labels. The index method was modified to accept this variable, rather then calling each
goto method individually.
 
Added variable:
<pre>
+ @@Groups = {
+  'Questionnaires' => 'Questionnaires',
+  'Review rubrics' => 'Review',
+  'Metareview rubrics' => 'Metareview',
+  'Teammate review rubrics' => 'Teammate Review',
+  'Author feedbacks' => 'Author Feedback',
+  'Global survey' => 'Global Survey',
+  'Surveys' => 'Survey',
+  'Course evaluations' => 'Course Evaluation',
+  'Courses' => 'Courses',
+  'Bookmarkrating' => 'Bookmarkrating',
+  'Assignments' => 'Assignments'
+ }
</pre>
 
Removed goto_assignments method:
- # direct access to assignments
- def goto_assignments
-  node_object = TreeFolder.find_by_name('Assignments')
-  session[:root] = FolderNode.find_by_node_object_id(node_object.id).id
-  redirect_to :controller => 'tree_display', :action => 'list'
- end
 
Added index method:
+ def index
+  session[:root] = params[:root]
+  group = getGroup session[:menu]
+  node_object = TreeFolder.find_by_name(group)
+  if not group.blank? and not node_object.blank?
+    session[:root] = FolderNode.find_by_node_object_id(node_object.id).id
+ end


==Use Routing Helpers==
==Use Routing Helpers==

Revision as of 20:32, 5 November 2014

E1463: Refactoring TreeDisplayController

Project Links

Our Expertiza Fork on AWS
Github Repo
Expertize Pull Request
Project Running on AWS
Admin Account login details :
Username: user2
Password: password

Project Description

For this project, our team refactored the TreeDisplayController class in the 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.

Example Image Of Tree Display:

Refactoring TreeDisplayController Tasks

As part of the refactoring task, we had to address the following issues for enhancing code readability and maintainability. The tasks include-

  1. Changing List to Index using a RESTful approach.
  2. Use of routing helpers
  3. Use {} and [] instead of Hash.new and Array.new
  4. Ensuring that instantiation of instance variables is minimized.


Changing List to Index

All our code changes can be checked out at our Git project repository.

Combined All of the Goto Methods

We also combined all of the goto methods into one, in accordance with the DRY principle. This removed a significant amount of duplicate code.

3 of the 10 unnecessary methods removed from tree_display_controller.rb

- # direct access to questionnaires 
- def goto_questionnaires 
- node_object = TreeFolder.find_by_name('Questionnaires') 
- session[:root] = FolderNode.find_by_node_object_id(node_object.id).id 
- redirect_to :controller => 'tree_display', :action => 'list' 
- end 
- 
- # direct access to review rubrics 
- def goto_review_rubrics 
- node_object = TreeFolder.find_by_name('Review') 
- session[:root] = FolderNode.find_by_node_object_id(node_object.id).id 
- redirect_to :controller => 'tree_display', :action => 'list' 
- end 
- 
- # direct access to metareview rubrics 
- def goto_metareview_rubrics 
- node_object = TreeFolder.find_by_name('Metareview') 
- session[:root] = FolderNode.find_by_node_object_id(node_object.id).id 
- redirect_to :controller => 'tree_display', :action => 'list' 
- end 

These methods were replaced by a variable containing a list mapping of all the session variable strings to database labels. The index method was modified to accept this variable, rather then calling each goto method individually.

Added variable:

+ @@Groups = { 
+   'Questionnaires' => 'Questionnaires', 
+   'Review rubrics' => 'Review', 
+   'Metareview rubrics' => 'Metareview', 
+   'Teammate review rubrics' => 'Teammate Review', 
+   'Author feedbacks' => 'Author Feedback', 
+   'Global survey' => 'Global Survey', 
+   'Surveys' => 'Survey', 
+   'Course evaluations' => 'Course Evaluation', 
+   'Courses' => 'Courses', 
+   'Bookmarkrating' => 'Bookmarkrating', 
+   'Assignments' => 'Assignments' 
+ } 

Removed goto_assignments method: - # direct access to assignments - def goto_assignments - node_object = TreeFolder.find_by_name('Assignments') - session[:root] = FolderNode.find_by_node_object_id(node_object.id).id - redirect_to :controller => 'tree_display', :action => 'list' - end

Added index method: + def index + session[:root] = params[:root] + group = getGroup session[:menu] + node_object = TreeFolder.find_by_name(group) + if not group.blank? and not node_object.blank? + session[:root] = FolderNode.find_by_node_object_id(node_object.id).id + end

Use Routing Helpers

We refactored the treedisplay controller class to use route helpers rather than calling ":action => 'list', :controller => 'tree_display'", or something similar. This involved many edits in the tree_display_controller.rb file and related controller and view files. Our Github repo shows all the changes made.

Breadcrumbs view file before:

redirect_to :action => 'list', :controller => 'tree_display'

After:

redirect_to direct_to tree_display_index_path

It can be seen that 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.

Minimizing the count of Instance Variables

Instance Variables in Ruby

An instance variable has a name beginning with @, and its scope is confined to whatever object self refers to. Two different objects, even if they belong to the same class, are allowed to have different values for their instance variables. From outside the object, instance variables cannot be altered or even observed (i.e., ruby's instance variables are never public) except by whatever methods are explicitly provided by the programmer. As with globals, instance variables have the nil value until they are initialized.

Instance variables do not need to be declared. This indicates a flexible object structure; in fact, each instance variable is dynamically appended to an object when it is first assigned.

We have observed that in Expertiza, there were several code files related to the Tree Display controller which were observed to be using multiple instance variables which could be reduced. Our team aimed to limit the instantiation of such variables to only necessary places in our code.

Other Methods

Summary and Conclusions

Future Work