HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Creating SOLID immutable and mutable Location types

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
creatingimmutabletypesandsolidmutablelocation

Problem

I have a Location type:

public abstract class Location implements Locatable {
    private int x, y;

    protected final void setX(int x) {
         this.x = x;
    }

    protected final void setY(int y) {
         this.y = y;
    }

    public final int getX() {
        return x;
    }

    public final int getY() {
        return y;
    }
}

interface Locatable {
    int getX();
    int getY();
}


Location has protected mutator methods, to ensure I can provide an MutableLocation type, but I'm not sure if this violates the Interface Seggregation principle when creating an ImmutableLocation.

I feel it doesn't, since the client will never have access to the setters through ImmutableLocation (cannot call it from an instance, cannot call it from a subclass of ImmutableLocation). So this was my attempt at a SOLID location type, which I plan on using in a project.

class ImmutableLocation extends Location {
    //...
 }

class MutableLocation extends Location {
    public void adjustX(int amount, IncremeantOperation op) {
        int x = op.adjust(x, amount);

        setX(x);
    }

    public void adjustY(int amount, IncremeantOperation op) {
        int y = op.adjust(getY(), amount);

        setY(y);
    }
}

enum IncremeantOperation {
    INCREASE {
        public int adjust(int target, int amount) {
             return target + amount;
        }
    },
    DECREASE {
        public int adjust(int target, int amount) {
            return target - amount;
        }
    };

    public int adjust(int target, int amount) {
        throw new UnsupportedOperationException("Cannot call adjust(int, int) on unknown operation");
    }
}


Please give your thoughts. My main concerns are:

  • I'm not sure if the enum is an overkill



  • If the setters are a violation of SOLID; I'm not sure how else I should do it.



  • If UnsupportedOperationException is best fit for the adjust method



```
interface Locatable {
Location getLocation();
}

public abst

Solution

It doesn't make sense to have setters in an immutable location.
Consider this alternative:

interface Location {
    int getX();
    int getY();
}

class MutableLocation implements Location {

    private int x;
    private int y;

    public MutableLocation(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }

    public void adjustX(int x) {
        //adjust x
    }

    public void adjustY(int y) {
        //adjust y
    }
}

final class ImmutableLocation implements Location {
    private final Location location;

    public ImmutableLocation(int x, int y) {
        location = new MutableLocation(x, y);
    }

    @Override
    public int getX() {
        return location.getX();
    }

    @Override
    public int getY() {
        return location.getY();
    }
}


Your questions



  • I'm not sure if the enum is an overkill




Not necessarily. Maybe it is, maybe it isn't. It will depend on the rest of the code in your project. From here it can go both ways.



  • If the setters are a violation of SOLID; I'm not sure how else I should do it.




They violate SOLID, in particular the interface segregation principle:
the immutable class inherits setters but doesn't need them.
See my proposed cleaner alternatives above.



  • If UnsupportedOperationException is best fit for the adjust method




For unsupported operations, this is fine, but your purpose is not clear.
Do you intend to add values in this enum without implementing adjust?
If yes, then your solution is fine.
But if you never intend to add values without implementing adjust,
then it would be better to make the method abstract.
Also you should use @Override in the implementations:

enum IncrementOperation {
    INCREASE {
        @Override
        public int adjust(int target, int amount) {
            return target + amount;
        }
    },
    DECREASE {
        @Override
        public int adjust(int target, int amount) {
            return target - amount;
        }
    };

    public abstract int adjust(int target, int amount);
}

Code Snippets

interface Location {
    int getX();
    int getY();
}

class MutableLocation implements Location {

    private int x;
    private int y;

    public MutableLocation(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }

    public void adjustX(int x) {
        //adjust x
    }

    public void adjustY(int y) {
        //adjust y
    }
}

final class ImmutableLocation implements Location {
    private final Location location;

    public ImmutableLocation(int x, int y) {
        location = new MutableLocation(x, y);
    }

    @Override
    public int getX() {
        return location.getX();
    }

    @Override
    public int getY() {
        return location.getY();
    }
}
enum IncrementOperation {
    INCREASE {
        @Override
        public int adjust(int target, int amount) {
            return target + amount;
        }
    },
    DECREASE {
        @Override
        public int adjust(int target, int amount) {
            return target - amount;
        }
    };

    public abstract int adjust(int target, int amount);
}

Context

StackExchange Code Review Q#90947, answer score: 4

Revisions (0)

No revisions yet.