CODEWITHSUNDEEP
javarefactoring
Pavel Petrashov
Pavel Petrashov

Reputation: 1262

Refactoring method with few boolean parameters

There is a good approach to refactoring. For example, we have this method with flag:

public void process(boolean hasLicense){
        if (hasLicense){
            System.out.println("has");
        }else {
            System.out.println("has not");
        }
    }

We can split it into two methods:

public void processWithLicense() {
    System.out.println("has");
}

public void processWithoutLicense() {
    System.out.println("has not");
}

But what about this type of method?

public void process(boolean hasLicense, boolean isAdmin) {
        if (hasLicense && isAdmin) {
            System.out.println("success");
        }
        
        System.out.println("some code");
        
        if (isAdmin){
            System.out.println("add grants");
        }

        System.out.println("10 lines code");
        
        if (isAdmin && !hasLicense){
            System.out.println("something");
        }
    }

How can I refactor this method and remove boolean parameters?

Upvotes: 1

Views: 942

Answers (1)

Andy Turner
Andy Turner

Reputation: 140329

How can I refactor this method and remove boolean parameters?

The thing about refactoring is that the decision strongly depends on what you're trying to do, and what the actual code look like: I can make suggestions, but these may or may not be suitable.

Something you could use here is the Template Pattern: you could define an interface something like:

interface Template {
  void part1();
  void part2();
  void part3();
}

and then refactor your method to look like:

public void process(Template template) {
    template.part1();
    
    System.out.println("some code");
    
    template.part2();

    System.out.println("10 lines code");
    
    template.part3();
}

and then pass in implementations of that interface for (admin/non-admin) x (license/no license).

Obviously this gets combinatorially large if you need to represent all of the possibilities, so you might not want to do this.

If the objection is to passing booleans, you could use another type. Effective Java makes the recommendation to pass two-element enums instead of booleans, for a number of reasons, including:

  • Being more descriptive: call sites are self-documenting where a literal value is used
  • Easier to add more options in the future
  • Being harder to get the order wrong, because the types of adjacent parameters are different
enum AdminState { IS_ADMIN, NOT_ADMIN }
enum LicenceState { HAS_LICENCE, NO_LICENCE }

public void process(AdminState adminState, LicenceState licenceState) {
    if (licenceState == HAS_LICENCE && adminState == IS_ADMIN) {
        System.out.println("success");
    }
    // ...
}

Alternatively, if it is specifically passing many arguments that you want to avoid, you could create a class to hold the many arguments:

class ProcessArgs {
  boolean isAdmin() { ... }
  boolean hasLicense() { ... }
}

and then pass that in:

public void process(ProcessArgs args) {
    if (args.hasLicense() && args.isAdmin()) {
        System.out.println("success");
    }
    // ...
}

This brings up the question of how an instance of ProcessArgs would be initialized: if you've simply got a constructor that takes the 2 booleans, it doesn't really help much in the goal of "not passing booleans", because you've just moved 2 booleans from a method to a constructor.

However, it does have the advantage of allowing you to pass the same set of arguments in multiple places, without worrying about things like transposing arguments.

Upvotes: 3

Related Questions