Design Pattern question: removing overuse of boolean function parameters-CodePudding

It's been decades since I've brushed up on the gang-of-four. I've lately been getting a bad smell from some code, and am looking for recommendations on optimal design.

There exists a function that services an API POST by accepting a binary upload, performing various processing on it, and storing a file. Over time, as new requirements have emerged, some of the steps in that function have needed to be skipped, based on the type of binary being uploaded. Over time, the method signature has evolved like so:

First iteration:

public ResponseObject uploadThing(long user_id, long location_id, byte[] file_bytes)

Second iteration:

public ResponseObject uploadThing(long user_id, long location_id, byte[] file_bytes, boolean ignore_azure)

Third iteration:

public ResponseObject uploadThing(long user_id, long location_id, byte[] file_bytes, boolean ignore_azure, boolean ignore_aws)

Fourth iteration:

public ResponseObject uploadThing(long user_id, long location_id, byte[] file_bytes, boolean ignore_azure, boolean ignore_aws, boolean log_to_airtable)

The booleans correspond to newly added conditionals wrapping the related sections of the function. There are multiple pathways into this function, so every time a new boolean is added, all the calling code needs to be revisited. Also, the code becomes less and less self-documenting. Here's an example of it in one place:

ro = uploadThing(user_id, org_id, file_bytes, true, false, true)

If any of the boolean-dependent code is actually triggered, the order of execution is critical. Also there's no information contained in the other parameters that could be used to determine which sections of the uploadThing method to execute - it's purely based on where the particular calling code is.

Some of the things I don't like about this: the increasingly tight coupling between caller and callee, the increasing need to cover multiple spots in a refactor, and the increasing obfuscation of the method call's intended behavior at the point where it's being called. How would you restructure this?

CodePudding user response：

You have an example of the telescopic arguments anti-pattern.

Use a builder pattern, especially when a feature, optionality, provision is added. Examples: the old HttpClient, the Base64 Encoder.

The problem here is you want a thin API, maybe even generated.

Use one map-like parameter class:

public ResponseObject uploadThing(UploadParams params)

public class UploadParams {
    private UploadParams() { }

    public static UploadParamsBuilder(long user_id, long location_id) { }

}

class UploadParamsBuilder {
    UploadParams build() { }
    UploadParamsBuilder withFileBytes(byte[] bytes) { }
    ...

The introduction could have been made already at the second iteration, as azure is en extra additional feature.

CodePudding user response：

Add an enum of flags, and pass them in with varargs.

ResponseObject uploadThing(long user_id, long location_id, byte[] file_bytes, Flags... flags)

Your final caller might look like

uploadThing(userId, orgId, fileBytes, Flags.IGNORE_AWS, Flags.LOG_TO_AIRTABLE);

CodePudding user response：

It is not very easy to give a general answer without knowing the requirements for your client code (the one that calls uploadThing).

Do all your calls have fixed boolean constants? If so, you could refactor the booleans into a separate configuration class and pass an object of it to uploadThing. Furthermore, if some of the code uses some of the boolean flags but not all, you could create an inheritance hierarchy for the configuration class, where in each subclass only the relevant flags are used.

Another solution could be to refactor uploadThing to a new class and include it via composition. In the constructor you could provide an appropriate combination of the booleans.

You could enhance these options to your taste (dependency injection, etc.)

But it really depends heavily on the requirements of the callee and how they use uploadThing.