CSC/ECE 517 Fall 2017/E1787 OSS project Bronze Score calculations

From Expertiza_Wiki
Jump to navigation Jump to search

Introduction

This project involves revision of score calculation bug of Expertiza homework reviewing mechanism.

Scenario

Sometimes when a reviewee of a project or homework fills out the review form, he or she may leave certain review question blank. When taken into score calculation for the project or homework, the application instead fills in 0 on blank answers. This behavior is incorrect as the blank review answers should never be used when used to calculate final score.

Issue -

Solution

Solution -

Github Link

Git Original link

Pull request link

Git Revised link

Explanation

Average Calculation

The scores are not calculated on dividing with total reviews given, instead calculated using total non null reviews given.


Design Pattern Usage

We implemented using the MVC (Model View Controller) pattern. We felt it was the pattern used in expertiza , usage of same pattern would be much easier.

Testing

We have run unit test using RSpec and Carpybara. The test file path is spec/models/vm_question_response_row_spec.rb.

For the test, we have identified 2 scenarios. First one is regarding correctness on average score calculation, where we will create a VmQuestionResponseRow object with sample test scores inside and call its average_score_for_row method, then we match returned average to the expected average (vm_q1 object);

Second one is regarding whether average score calculation includes nil values, where we will create a VmQuestionResponseRow object with one or more nil sample test scores inside and call its average_score_for_row method, then we match returned average to the expected average to check if nil is redefined and used in score calculation (vm_q2 object).

In addition, we have also tested the initialization of VmQuestionResponseRow object with 5 parameters and 6 parameters to ensure the initialization process works as intended (see in code for vm_q1 and vm_q3 objects). However, due to decrease in coverage for the test (after we added it, the coverage dropped by 0.4%), we have reverted it back to original 2 test cases. The additional test cases can be found below in snapshots, and original test cases with highest coverage in on github.

test file: vm_question_response_rows_spec.rb;

Our Work

For the OSS project, the topic is fixing and modification on Expertize team score calculation mechanism. For this project, a meeting once every week for 2 weeks with TA was proceeded. The main focus for the first one and half week involves setting up the environment on the machine. Some of notable issues on setting up include theRubyRacer dependency problem on project, Node packages not recognized by rails (Windows), Font-Awesome-Rails path issues, etc, which were solved through file tracing, problem identifying, and debugging. For the rest of these a little over 2 weeks period, several issues were identified with the project. The main issue for program include the false score calculation mechanism by expertiza where empty scores are assumed automatically to 0 and included in calculation; this would in turn give a false score calculation result as empty scores are not supposed to be set to 0 and then included in for calculation. Through debugging and defining the problem, it was identified that the reason for such issue involves average_score_for_row method in vm_question_response_row.rb in model as shown below in red - where the row_average_score dividend was not been used correctly.

Comparison of code changes can be seen in Pull request.

The issue can be solved by modify the calculation for dividend using a counter to exclude nil values and modified the constructor for VmQuestionResponseRow classs that it can dynamically accept 5 or 6 parameters, as shown below (note: not_null_reviews.zero? check are used two times in order to pass the github check on coding, as combining these two lines into one (row_average_score = (row_average_score / not_null_reviews).round(2) unless not_null_reviews.zero?) would resulting in failing on github check, while the other solution which is putting both line into an unless block would not have any benefit execution efficiency wise).

Code after first modification -

Code after second modification -

The first modification was done as it fix the issue while keep code at its most simplicity; (As shown in screen shot)

The second modification was done to accommodate the scenario where an extra parameter - score row - is needed during object initialization process. It would be a more efficient constructor to use when retrieving scores from database as it allows program to initialize a VmQuestionResponseRow object now with a list of AssignmentScore objects instead of having to initialize to an empty list and putting in AssignmentScore objects one by one through a loop. (Note: the implementation for using constructor with extra parameter in project is not implemented as it is originally not part of our requirement)

Test implementation

Afterward, several tests were performed for the model class modifications using RSpec, with some of the code as shown below:


For this project, techniques such as Unit testing, constructor overloading, and technologies such as bower, carpybara, RSpec, etc, were used.