Design review for refactoring a large function

Question

Basic terminology:
Report - single report to generate
ReportGroup - group of Report(s) to generate
Request - contains one or more ReportGroup

I'm working on refactoring of the large function (~350 lines) in an even larger class (~2500 lines)
The function is responsible for a generation of report in various formats (txt,csv, xlsx, pdf, zip)
Currently, function supports extraction for a single Report or single ReportGroup. My task is to extend the function to support extraction for multiple ReportGroups.

This function is essensially a series of steps that eventually generate a file in requested format.
Below are the steps I could identify for a single report:

1. Analyze/Validate input - return if not valid.
2. Get result (In form of a tree) - return if not valid.
3. Create ResultDescriptor - Poco containing tree result and additional meta data calculates from request and result properties.
4. Create GenerationResultStatus and add it to the Response object
5. Pass result descriptor to the rendering engine and get the path to file back.
6. Move generated file to shared location (location is determined based on ResultDescriptor properties).
7. Generate Response object containing end result file location, GenerationResultStatus, overallstatus (bool) and error message and return it to the caller.

I've came up with the following classes:
Single report generation actors:

ResultDescriptorBuilder
GenerationResultStatusBuilder
StandartizedFileMover
ResponseBuilder

Report generators:

SingleReportGenerator - processes a single report and outputs a single file, GroupedReportGenerator - Decorator on a SingleReportGenerator. Processes report group and outputs a single file (xlsx, pdf).
ZipedReportGenerator - Decorator on a SingleReportGenerator. Processes report group and outputs a single zip file .
GroupOfGroupedReportGeneratorPdf - Decorator on GroupedReportGenerator. Processes the whole request by looping each ReportGroup and passing it to GroupedReportGenerator.
The resulting files are then merged to a single pdf. Supports only pdf format.

I guess, the biggest question is should I even bother doing the refactoring (from a purely practical point of view)?
I could probably get away with a patch that calls current function in a loop and then merges the results.
The function doesn't change very often and I don't foresee any new changes.
I do have some unit and integration tests in place, but they're pretty basic and don't offer full coverage.

In case I do decide to go with the refactoring, Are there any issues I overlooked?
Is there a design pattern that can make this more generic?
Any naming suggestions are welcome. I think I overuse the Generator/Builder words.

Answer 1

Your way of thinking is perfectly reasonable.

I guess, the biggest question is should I even bother doing the refactoring (from a purely practical point of view)?

That depends. From the artist's standpoint, the answer is YES. But there is money involved. So, you must estimate the impact of unrefactored code to your development cycle. If it will cause difficulties or damage system design in any way, the answer is again YES.

I could probably get away with a patch that calls current function in a loop and then merges the results.

Probably. Actually, that would make some facade over current code, that will just hide some ugly piece of it ;)

In case I do decide to go with the refactoring, Are there any issues I overlooked? Is there a design pattern that can make this more generic?

I don't think so.

• DanielS: Thanks for your feedback. I know it's hard thing to do without seeing the actual code.
• Vladislav Rastrusny: @DanielS Welcome :)