intellij-ideacoding-stylerefactoringautomated-refactoringparameter-object

What are the advantages of extracting parameter objects?


There is a refactoring tool in IntelliJ-IDEA which allows me to extract a parameter object from a method.

enter image description here

This will do something like the following:

public interface ThirdPartyPoint {
    float getX();
    float getY();
}

Before:

class Main {
    public static float distanceBetween(float x1, y1, x2, y2) {
        return distanceBetween(x1, y1), x2, y2);
    }

    public static float distanceBetween(ThirdPartyPoint point1, ThirdPartyPoint point2) {
        return distanceBetween(point1.getX(), point1.getY(), point2.getX(), point2.getY());
    }
}

After:

class Main {

    public static float distanceBetween(Point point1, Point point2) {
        return Math.sqrt(Math.pow(point2.getX() - point1.getX(), 2) + Math.pow(point2.getY() - point1.getY(), 2));
    }

    public static float distanceBetween(ThirdPartyPoint point1, ThirdPartyPoint point2) {
        return distanceBetween(new Point(point1.getX(), point2.getY()), new Point(point2.getX(), point2.getY()));
    }

    private static class Point {
        private final float x;
        private final float y;

        private Point(float x, float y) {
            this.x = x;
            this.y = y;
        }

        public float getX() {
            return x;
        }

        public float getY() {
            return y;
        }
    }
}

Why is this any better than before?

Now, if I have to use this method, I need to create a new point object each time I invoke it. Whereas before, I could just use the primitive types.

My feeling is that method signatures should generally go in the opposite direction. For example, if you have some function that finds out how popular a name is like this:

public int nameRanking(String name) {
    // do something with name
}

And you extract a parameter object like this:

public int nameRanking(Person person) {
    // do something with person.getName()
}

Doesn't that make things worse? For example, what if, after creating the Person class from the refactoring menu, I decide to remove the getName() method because I don't want the name to be publicly available for all persons, but other classes used the nameRanking function? Now I need to change my nameRanking function. If I used the built-in String class, I know that nothing injected into this function would ever change.


Solution

  • I consider your second example (with name and Person) to be very different from your first example. In the first example, you are not sending more or less information, you're sending the exact same information in a different form. In your second example, as you noted, you end up sending a lot of information the function does not need to perform it's duty. As a rule of thumb, you always want to limit the information you are sending a function to what it needs to do perform it's tasks. In your case, that is the name of the person. That helps reduce coupling and keeps your functions from having to know about too many other class's behavior/structure to perform it's task. You could make a case that your name ranking function might someday be used to make rankings weighted with age groups and other information and that for this reason, it would be better to send a whole Person object, but that is not the case now, so I won't be getting into that.

    Let's go back to your first example. As I've mentioned, in that case, you end up sending the same information, in another form. In which case would that be better than sending a longer list of primitives? Let's say we're coding Red Square. In this game, you have to control a red square around and dodge the moving blue rectangles. I'll present you two different ways in which some of the code could be written.

    The first implementation could look like this:

    int x2 = game.player.pos_x
    int y2 = game.player.pos_y
    int w2 = game.player.width
    int h2 = game.player.height
    for(Enemy enemy : game.enemies) {
        int x2 = enemy.pos_x
        int y2 = enemy.pos_y
        int w2 = enemy.width
        int h2 = enemy.height
    
        // If it is colliding with the player
        if(collides(x1, y1, w1, h1, x1, y2, w2, h2)) {
            die()
        }
    
        // If it is going out of bounds, bump it back inside
        if(isInBounds(0, 0, game.map.width, game.map.height, x2, y2, w2, h2)) {
            enemy.bump()
        }
    
        //... More functions that use x1, y1, w1, w1, ...
    }
    

    Here is a second implementation, where we've introduced a Parameter Object, Rectangle:

    Rectangle playerRect = game.player.rect
    for(Enemy enemy : game.enemies) {
        Rectangle enemyRect = enemy.rect
    
        // If it is colliding with the player
        if(collides(playerRect)) {
            die()
        }
    
        // If it is going out of bounds, bump it back inside
        if(isInBounds(game.map.rect, enemyRect)) {
            enemy.bump()
        }
    
        //... More functions that use playerRect and enemyRect
    }
    

    Some advantages of the second implementation includes:

    1. More readable function calls (less parameters)
    2. Less room for errors with sending parameters out of order (what if I send (width, height, x, y) instead of (x, y, width, height)?)
    3. You're sure to always work with whole instances of rectangles (no chance of sending a function your player's x instead of the enemy's x, or mistakes like that)

    I believe your first example with Point is much more similar to the Rectangle one than your second example (where you end up injecting tighter coupling to information you don't need). If you have a group of parameters that you often send out together to different functions, it might be a sign that they are acting as one entity, and would benefit from the "Introduce Parameter Object" refactoring. For example, in your Point example, you rarely (never?) use x and y as independent information. You always use them as one entity, a point (x,y).

    Keep in mind you always have to be careful when using automated-refactoring tools. "Introduce Parameter Object" should not be used blindly on every single function call. As I've said earlier, you should ask yourself if it makes sense (conceptually) to bundle those parameters as one single entity. With that being said, that refactoring can help make your code more readable and reduce the number of parameters you have to send a function, resulting in less mistakes in the order or mismatching of parameters. Don't believe me? Have you noticed the subtle mistake in the first collides(...) function call? :-)