CSC/ECE 517 Fall 2024 - E2459. View for results of bidding

Introduction

Expertiza is an open-source platform educational institutions use to create, assign, and manage assignments and peer assessments. To improve its functionality, our team enhanced the existing implementation of feature E2459 (previously E2410) view results of bidding for the Expertiza application. The goal of the original project aimed to resolve the issues where instructors could only view the topic bidding process by impersonating students, and they needed to determine if the bidding assignment algorithm process was assigning teams to their selected topics. 

Our objectives for this enhancement were:

• Rename methods to align with Ruby on Rails naming conventions
• Optimize the table layout to use space efficiently to display the bidding information
• Make the code cleaner
• Enhance test cases with appropriate comments

Primary Design Principle: Single Responsibility Principle (SRP)

One of the things that we did to make the code cleaner was to start by looking for opportunities to improve bad design choices in the original implementation. One thing that stood out was the need for adherence to the Single Responsibility Principle (SRP). We chose to start with SRP mainly because we can ensure that each method in the business logic has a clear purpose and is not confusing because it is trying to do too much. We also wanted to make the code easier to maintain in the future.

With the original implementation, multiple business logic methods handled multiple responsibilities. This violated SRP, which states, "Your class or method should have only one reason to change." By applying the SRP design principle, we could break the original methods into more focused ones that handled a single responsibility.

Problem Statement

1. Controller Naming Issue: The name LotteryController is misleading as it suggests random assignment rather than the automatic team assignment based on k-means clustering and bidding data.
2. Embedded Business Logic: The run_intelligent_assignment and the calculate_bidding_summary_based_on_priority methods in the original LotteryController contain embedded business logic through private methods, violating the Single Responsibility Principle (SRP) and leading to a bloated controller.
3. Performance Inefficiency: Multiple database queries are being executed within loops, resulting in inefficient performance and increased load on the database.

Our Changes

The refactoring effort streamlines the Bidding Controller (formerly Lottery Controller) by moving business logic to dedicated service objects. By ensuring that the controller's sole focus is to handle HTTP requests and responses, we have enhanced code clarity and maintainability and ensured adherence to the Single Responsibility Principle.

Refactor LotteryController to Remove Business Logic and Only Handle HTTP Requests and Responses

#	Change	Rationale	Commit Link
1	Introduction of TeamAssignmentService as a separate service object.	We created a new service object called TeamAssignmentService. We moved the business logic related to automatically assigning teams to topics out of the controller object (now called BiddingController) so that the controller can focus on handling HTTP requests and responses. This move no longer bloats the controller with business logic methods it should not be concerned with. It separates the business logic concern into a separate layer that makes sense, and overall, it is a cleaner way to view all business logic related to team assignments.	Commit
2	Introduction of BiddingSummaryService as a separate service object.	We created a new service object called BiddingSummaryService. We moved the business logic out of the controller object (now called BiddingController) so that the controller can focus on handling HTTP requests and responses. This move no longer bloats the controller with business logic methods it should not be concerned with. It separates the business logic concern into a separate layer that makes sense, and overall, it is a cleaner way to view all business logic related to calculating bid summary.	Commit
3	Applying SRP to Methods Handling Multiple Responsibilities	Majority of the methods in the business logic that was formerly under the LotteryController were handling multiple responsibilities in a single method. This makes the code harder to test and maintain. Now, after refactoring the business logic to the TeamAssignmentService object, we have separate and direct methods for retrieving bids from the database, constructing the teams and their respective bids, and sorting the bids so that each method handles a single responsibility, is scalable and adaptable to future code changes, and is easy to test. Even though this resulted in more lines of code, it is still a better approach since it aligns with SRP.	Commit

Remove Inline Scripts and Styling from Views and Update Table Layout for Project Bidding Statics

#	Change	Rationale	Commit Link
1	Move inline styles from _topics.html.erb partial view to assignment_topics.scss	Removing inline scripts and styles from views aligns with SRP, extracting the styling code out of the view to centralize styling and make the code cleaner and more manageable.	Commit
2	Move inline styles from lottery views to bidding_summary.scss	Removing inline scripts and styles from views aligns with SRP, extracting the styling code out of the view to centralize styling and make the code cleaner and more manageable.	Commit
3	Move inline scripts from _topics.html.erb partial view to assignment_topics.js	Removing inline scripts and styles from views aligns with SRP, extracting the JavaScript logic out of the view to centralize the scripts and make the code cleaner and more manageable.	Commit
4	Update Table Layout by narrowing down column widths, and adding info buttons	Although the original view provided a clear overview, it could be made better by resizing the table and providing additional information on the columns	Commit

Rename Methods to Follow Rails Naming Conventions

#	Change	Rationale	Commit Link
1	Changed the name of the controller from LotteryController to BiddingController	In reviewing the original implementation, one of the first things we considered was the name of the LotteryController. The term "Lottery" did not seem to accurately reflect its functionality, as it appears to focus more on automatically assigning teams rather than randomly assigning them. This is especially true since the method sends a request to a web service that uses k-means clustering—a deterministic algorithm—and students' bidding data. Additionally, it attempts to create teams based on desired sizes. Based on this, we wanted to ensure that the controller's purpose is clear from its name and eliminate ambiguity. The original implementation named the controller LotteryController because the initial design was to implement a chance-based lottery for assigning students to topics. However, it evolved to allow students to specify their bids.	Commit
2	Rename run_intelligent_assignment method in bidding_controller.rb	Renaming the run_intelligent_assignment method to auto_assign_teams adheres to the Rails Naming Convention because auto_assign_teams clearly describes what the method is doing whereas run_intelligent_assignment was more vague.	Commit

Tests

construct_teams_bidding_info

This method constructs a bidding information hash based on provided unassigned teams and sign-up topics. The tests ensure the method behaves correctly under various scenarios as follows:

1. Test Case 1: Generate teams bidding info hash based on newly created teams
   • Purpose: Verify that the method correctly generates a hash containing bidding information for each team when provided with valid unassigned teams and sign-up topics.
   • Expected Output: An array of hashes with a size equal to the number of unassigned teams
2. Test Case 2: Returns an empty array if no unassigned teams are provided
   • Purpose: Ensure that the method returns an empty array when no unassigned teams are given, regardless of sign-up topics.
   • Expected Output: An empty array ([]).
3. Test Case 3: Calls the fetch_bids and construct_team_bids methods
   • Purpose: Confirm that the fetch_bids and construct_team_bids methods are invoked during the execution of construct_teams_bidding_info.
   • Expected Output: Both fetch_bids and construct_team_bids methods are called with the correct arguments.

Example Test Login Credentials

UserId: instructor6

Password: password

Opportunities for Additional Future Enhancements

• Refactor is_intelligent property for assignment to uses_bid for additional clarity. is_intelligent doesn't make it clear as to what the true purpose of the function is. uses_bid makes more sense.
• Add test cases for other service methods, prioritizing unit tests on public methods.
• Additional refinement and optimization of methods in the BiddingSummaryService.

Team

Mentor

• Edward F. Gehringer

Members

• Janice Uwujaren
• Aarya Rajoju
• Jack Henza

Links

• Demo Video Link
• Pull Request

References

• Project Instructions
• Previous Implementation