Best design patterns for refactoring code without breaking other parts

Question

I have some PHP code from an application that was written using Laravel. One of the modules was written quite poorly. The controller of this module has a whole bunch of reporting functions which uses the functions defined inside the model object to fetch data. And the functions inside the model object are super messy.

Following is a list of some of the functions from the controller (ReportController.php) and the model (Report.php)(I'm only giving names of functions and no implementations as my question is design related)

Functions from ReportController.php

questionAnswersReportPdf()
fetchStudentAnswerReportDetail()
studentAnswersReport()
wholeClassScoreReportGradebookCSV()
wholeClassScoreReportCSV()
wholeClassScoreReportGradebook()
formativeWholeClassScoreReportGradebookPdf()
wholeClassScoreReport()
fetchWholeClassScoreReport()
fetchCurriculumAnalysisReportData()
curriculumAnalysisReportCSV()
curriculumAnalysisReportPdf()
studentAnswersReportCSV()
fetchStudentScoreReportStudents()

Functions from Report.php

getWholeClassScoreReportData
getReportsByFilters
reportMeta
fetchCurriculumAnalysisReportData
fetchCurriculumAnalysisReportGraphData
fetchCurriculumAnalysisReportUsersData
fetchTestHistoryClassAveragesData
fetchAllTestHistoryClassAveragesData
fetchAllTestHistoryClassAveragesDataCsv
fetchHistoryClassAveragesDataCsv
fetchHistoryClassAveragesGraphData

The business logic has been written in quite a messy way also. Some parts of it are in the controller while other parts are in the model object.

I have 2 specific questions :

a) I have an ongoing goal of reducing code complexity and optimizing code structure. How can I leverage common OOP design patterns to ensure altering the code in any given report does not negatively affect the other reports? I specifically want to clean up the code for some critical reports first but want to ensure that by doing this none of the other reports will break.

b) The reporting module is relatively static in definition and unlikely to change over time. The majority of reports generated by the application involve nested sub-queries as well as standard grouping & filtering options. Most of these SQL queries have been hosed within the functions of the model object and contain some really complex joins. Without spending time evaluating the database structure or table indices, which solution architecture techniques would you recommend for scaling the report functionality to ensure optimized performance? Below is a snippet of one of the SQL queries

    $sql = 'SELECT       "Parent"."Id",
                "Parent"."ParentId",
                "Parent"."Name" as systemStandardName,
                string_agg(DISTINCT((("SubsectionQuestions"."QuestionSerial"))::text) , \', \') AS "quesions",
                count(DISTINCT("SubsectionQuestions"."QuestionId"))                        AS "totalQuestions",
                case when sum("SQUA"."attemptedUsers")::float > 0 then
                        (COALESCE(round((
                            (
                                sum(("SQUA"."totalCorrectAnswers"))::float
                                /
                                sum("SQUA"."attemptedUsers")::float
                            )
                            *100
                        )::numeric),0))
                    else 0 end as classacuracy,
                case when sum("SQUA"."attemptedUsers")::float > 0 then
                (COALESCE((round(((1 -
                    (
                        (
                            sum(("SQUA"."totalCorrectAnswers"))::float
                            /
                            sum("SQUA"."attemptedUsers")::float
                        )
                    )
                )::float * count(DISTINCT("SubsectionQuestions"."QuestionId")))::numeric,1)),0))
                else 0 end as pgain
        FROM       "'.$gainCategoryTable.'" as "Parent"
                    '.$resourceTableJoin.'
        INNER JOIN "SubsectionQuestions"
        ON         "SubsectionQuestions"."QuestionId" = "resourceTable"."ResourceId"
        INNER JOIN "Subsections"
        ON         "Subsections"."Id" = "SubsectionQuestions"."SubsectionId"
        LEFT Join (
                Select "SubsectionQuestionId",
                count(distinct case when "IsCorrect" = \'Yes\' then CONCAT ("UserId", \' \', "SubsectionQuestionId") else null end) AS "totalCorrectAnswers"
                , count(distinct CONCAT ("UserId", \' \', "SubsectionQuestionId")) AS "attemptedUsers"
                From "SubsectionQuestionUserAnswers"';
    if(!empty($selectedUserIdsArr)){
        $sql .=   ' where "UserId" IN (' .implode (",", $selectedUserIdsArr).')' ;
    }else {
        $sql .=   ' where "UserId" IN (' .implode (",", $assignmentUsers).')' ;
    }

    $sql .= ' AND "AssessmentAssignmentId" = '.$assignmentId.' AND "SubsectionQuestionId" IN ('.implode(",", $subsectionQuestions).') Group by "SubsectionQuestionId"
        ) as "SQUA" on "SQUA"."SubsectionQuestionId" = "SubsectionQuestions"."Id"
        INNER JOIN "AssessmentAssignment"
        ON "AssessmentAssignment"."assessmentId" = "Subsections"."AssessmentId"
        INNER JOIN "AssessmentAssignmentUsers"
        ON "AssessmentAssignmentUsers"."AssignmentId" = "AssessmentAssignment"."Id"
        AND "AssessmentAssignmentUsers"."Type" = \'User\'
                    '.$conditaionlJoin.'
        WHERE      "Parent"."Id" IN ('.implode(',', $ssLeaf).') 
                    '.$conditionalWhere.'
        GROUP BY   "Parent"."Id",
                    "Parent"."ParentId",
                    "Parent"."Name"
        '.$sorter;

    $results = DB::select(DB::raw($sql));

Answer 1

about a) : The first thing I do, to ensure none of my refactoring is breaking anything, is covering the code with, at least, unitary tests (doing TDD ensures the most optimal coverage). It's easier when, like @DavidY says, you respect principles like SRP (does my class try to answer too many problems ?). With test, you'll feel safer when you'll need to refactor, and the tests will tell you exactly where it broke.

about b) : Do not optimize until you need it. And optimize only when you know what cost you the most. It's the best way to know what pattern you need, otherwise you may try to force the wrong solution into the wrong problem.

Answer 2

My take on a). In my experience when I want to reduce code complexity/sheer messiness I slowly refactor out code that violates the single responsibility principle while I'm working in that area already for either a bug fix or a feature update. I try not to spend hours upon hours of updating code that is "working" that I'm not actively updating for a business process reason. Follow the "Leave it better than you found it" approach as you work in this code base, and it will get better over time. Doing this will allow you to improve the code base while also getting features and bug fixes out the door, while also keeping business owners/project managers happy because you're keeping things moving.