CSC/ECE 517 Fall 2015/oss E1560 PSV: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
mNo edit summary
mNo edit summary
Line 66: Line 66:
|- style="vertical-align:top;"
|- style="vertical-align:top;"
|<pre>
|<pre>
   
  def team_users_popup
    @sum = 0
    @count = 0
    @teamid = params[:id]
    @team = Team.find(params[:id])
    @assignment = Assignment.find(@team.parent_id)
    @assignment_id = @assignment.id
    @id=params[:assignment_id]
    #  @teamname = Team.find(params[:id]).name
    @teamusers = TeamsUser.where(team_id: params[:id])
 
    #id2 seems to be a response_map
    if(params[:id2] == nil)
      #  if(@reviewid == nil)
      @scores = nil
    else
      #get the last response from response_map id
      response = Response.where(map_id:params[:id2]).last
      @reviewid = response.id
      @pid = ResponseMap.find(params[:id2]).reviewer_id
      @reviewer_id = Participant.find(@pid).user_id
 
      @scores = Answer.where(response_id: @reviewid)
 
      questionnaire =Response.find(@reviewid).questionnaire_by_answer(@scores.first)
      @maxscore = questionnaire.max_question_score
 
      if(@maxscore == nil)
        @maxscore = 5
      end
 
      @total_percentage = response.get_average_score
      @sum = response.get_total_score
      @total_possible = response.get_maximum_score
    end
 
    #    @review_questionnaire = Questionnaire.find(@assignment.review_questionnaire_id)
    #    @review_questions = @review_questionnaire.questions
    #@maxscore = @review_questionnaire.max_question_score
 
 
end
</pre>
</pre>
|<pre>
|<pre>
def team_users_popup
    @sum = 0
    @team = Team.find(params[:id])
    @id = params[:assignment_id]
    @teamusers = TeamsUser.where(team_id: params[:id])
    puts params
    # response_map id can be used to index into the Response
    # table and find the desired response.
    if (params[:response_map_id] == nil)
      @scores = nil
    else
      #get the last response from response_map id
      response = Response.where(map_id: params[:response_map_id]).last
      @reviewid = response.id
      @pid = ResponseMap.find(params[:response_map_id]).reviewer_id
      @reviewer_id = Participant.find(@pid).user_id
      @scores = Answer.where(response_id: response.id)
      questionnaire = Response.find(@reviewid).questionnaire_by_answer(@scores.first)
      if (questionnaire.max_question_score == nil)
        @maxscore = 5 # This instance variable is needed. It's used in the team_users_popup view.
      end


      @total_percentage = response.get_average_score
      @sum = response.get_total_score
      @total_possible = response.get_maximum_score
    end
end
</pre>
</pre>
|}
|}

Revision as of 06:20, 31 October 2015

E1560. Refactoring PopUpController.rb and ParticipantsController.rb

This page provides a description of the Expertiza based OSS project. This project is aimed at refactoring PopUpController.rb and ParticipantsController.rb.


Introduction to Expertiza

Expertiza is an Open Source Rails application which is used by instructors and students for creating assignments and submitting peer reviews. Expertiza allows the instructor to create and customize assignments, create a list of topics the students can sign up for, have students work on teams and then review each other's assignments at the end. The Source code of the application can be cloned from Github.


Project Desicription<ref>https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus/edit</ref>

Files involved:

popUpController.rb

participantsController.rb

What they do

PopUpController displays a review when a user (instructor or student) clicks on a list of reviews.

ParticipantsController handles participants in an assignment.

What's wrong with them:

PopUpController contains only two prominent methods, but they are very big.

ParticipantsController contains redundant code.

What needs to be done:

1 popUpController.rb

1.1 Action_allowed method always returns true. It needs to be fixed.

1.2 Team_user_popup method needs refactoring. @teamId is assigned but never used. Add comments to make code more readable. Rename variables based on their usage

1.3 Instance variables need to be changed to local variable wherever possible.

1.4 Refactor Participants_popup and team_users_popup methods into smaller private methods.

2 participantsController.rb

2.1 In add and update_authorization methods, permissions collection object can be used to reference its elements without assigning each element to individual private variables.

2.2 Inherit and bequeath_all methods are similar. Common statements can be migrated to private method. Add comments in code to make it easy to understand.

2.3 Cluster all the flash messages under one private method to make the code more manageable

2.4 Fix email_sent method. It contains a dummy email address.

Changes Made<ref>https://github.com/viveksubbarao/expertiza/commits/master</ref>

PopUpController.rb

Case 1.1: Modified Action_allowed method to return true only if ..

Before Refactoring After Refactoring 
def action_allowed?
    true
end

def action_allowed?
    if @allowed_actions.include? params[:action]
        true
    else
        false
    end
end

Case 1.2: Refactored Team_user_popup method

Before Refactoring After Refactoring 
 def team_users_popup
    @sum = 0
    @count = 0
    @teamid = params[:id]
    @team = Team.find(params[:id])
    @assignment = Assignment.find(@team.parent_id)
    @assignment_id = @assignment.id
    @id=params[:assignment_id]
    #  @teamname = Team.find(params[:id]).name
    @teamusers = TeamsUser.where(team_id: params[:id])

    #id2 seems to be a response_map
    if(params[:id2] == nil)
      #  if(@reviewid == nil)
      @scores = nil
    else
      #get the last response from response_map id
      response = Response.where(map_id:params[:id2]).last
      @reviewid = response.id
      @pid = ResponseMap.find(params[:id2]).reviewer_id
      @reviewer_id = Participant.find(@pid).user_id

      @scores = Answer.where(response_id: @reviewid)

      questionnaire =Response.find(@reviewid).questionnaire_by_answer(@scores.first)
      @maxscore = questionnaire.max_question_score

      if(@maxscore == nil)
        @maxscore = 5
      end

      @total_percentage = response.get_average_score
      @sum = response.get_total_score
      @total_possible = response.get_maximum_score
    end

    #    @review_questionnaire = Questionnaire.find(@assignment.review_questionnaire_id)
    #    @review_questions = @review_questionnaire.questions
    #@maxscore = @review_questionnaire.max_question_score


 end

 def team_users_popup
    @sum = 0
    @team = Team.find(params[:id])
    @id = params[:assignment_id]
    @teamusers = TeamsUser.where(team_id: params[:id])

    puts params
    # response_map id can be used to index into the Response
    # table and find the desired response.
    if (params[:response_map_id] == nil)
      @scores = nil
    else
      #get the last response from response_map id
      response = Response.where(map_id: params[:response_map_id]).last
      @reviewid = response.id
      @pid = ResponseMap.find(params[:response_map_id]).reviewer_id
      @reviewer_id = Participant.find(@pid).user_id

      @scores = Answer.where(response_id: response.id)
      questionnaire = Response.find(@reviewid).questionnaire_by_answer(@scores.first)

      if (questionnaire.max_question_score == nil)
        @maxscore = 5 # This instance variable is needed. It's used in the team_users_popup view.
      end

      @total_percentage = response.get_average_score
      @sum = response.get_total_score
      @total_possible = response.get_maximum_score
    end
 end

Case 1.3: Redundant instance variables changed to local variables

Before Refactoring After Refactoring

Case 1.4: Refactored Participants_popup and team_users_popup methods into smaller private methods

Before Refactoring After Refactoring

ParticipantsController.rb

Case 2.1: Permissions collection object referenced directly in add and update_authorization methods

Before Refactoring After Refactoring 
def add
......
    permissions = Participant.get_permissions(params[:authorization])
    can_submit = permissions[:can_submit]   
    can_review = permissions[:can_review]
    can_take_quiz = permissions[:can_take_quiz]
    curr_object.add_participant(params[:user][:name],can_submit,can_review,can_take_quiz)
......
end

def update_authorizations
......
    permissions = Participant.get_permissions(params[:authorization])
    can_submit = permissions[:can_submit]
    can_review = permissions[:can_review]
    can_take_quiz = permissions[:can_take_quiz]
    participant = Participant.find(params[:id])
    parent_id = participant.parent_id
    participant.update_attributes(:can_submit => can_submit, :can_review => can_review, :can_take_quiz => can_take_quiz)
......
end

def add
......
    permissions = Participant.get_permissions(params[:authorization])
    curr_object.add_participant(params[:user][:name], permissions[:can_submit], 
              permissions[:can_review], permissions[:can_take_quiz])
......
end


def update_authorizations
......
    permissions = Participant.get_permissions(params[:authorization])
    participant = Participant.find(params[:id])
    parent_id = participant.parent_id
    participant.update_attributes(:can_submit => permissions[:can_submit], 
           :can_review => permissions[:can_review], :can_take_quiz => permissions[:can_take_quiz])
......
end

Case 2.2: Moved common functionality in Inherit and bequeath_all to private methods

Before Refactoring After Refactoring

Case 2.3: Flash messages clustered under a single private method

Before Refactoring After Refactoring

Case 2.4: Fixed email_sent method

Before Refactoring After Refactoring

References

<references/>

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2015/oss_E1560_PSV&oldid=97491"