skiwi
skiwi

Reputation: 69279

Java class accepting different objects on same argument in one constructor

This is my old code and I decided to refactor it in order to make refuse easier, but now I wonder if it can be done even simpler.

public class AccountConstraint {
    private Range<Integer> accountId;
    private String username;
    private String password;
    private String email;

    public AccountConstraint(Object accountId, final String username, final String password, final String email) {
        if (accountId instanceof Integer) {
            accountId = new Range<>(accountId);
        }
        this.accountId = (Range<Integer>)accountId;
        this.username = username;
        this.password = password;
        this.email = email;
    }
}

public class Range<T> {
    private T min;
    private T max;

    public Range(T min, T max) {
        this.min = min;
        this.max = max;
    }

    public Range(T value) {
        this.min = value;
        this.max = value;
    }

    public T getMin() {
        return min;
    }

    public T getMax() {
        return max;
    }

    @Override
    public String toString() {
        return "[" + min + ", " + max + "]";
    }
}

With this you can create an AccountConstraint that has as accountId either an Integer or a Range<Integer> and ensures that it is always saved as a Range<Integer>.

Background information here is that you should be able to just enter an integer like 5 and that it will be automatically stored as Range(5) = Range(5, 5) then.

The code works fine, but it seems a bit messy. So I came up with the following improvement:

New constructor:

public AccountConstraint(Object accountId, final String username, final String password, final String email) {
    this.accountId = Range.<Integer>getRange(accountId, Integer.class);
    this.username = username;
    this.password = password;
    this.email = email;
}

New method:

public static <T> Range<T> getRange(Object object, Class<? extends T> clazz) {
    if (clazz.isInstance(object)) {
        object = new Range<>((T)object);
    }
    return (Range<T>)object;
}

It also works and it seems better in my opinion as it can be done as a one-liner now, but can it even be done better? More specifically, can the Class<? exends T> be left out?

And a small question: Should it be Class<T> clazz or Class? extends T> clazz?

Regards.

EDIT: Implemented the Builder Pattern as suggested, because there is a possibility that there will be more arguments that can be overloaded. New code:

package dao.constraint;

public class AccountConstraint {
    private Range<Integer> accountId;
    private String username;
    private String password;
    private String email;

    private AccountConstraint(Builder builder) {
        this.accountId = builder.accountId;
        this.username = builder.username;
        this.password = builder.password;
        this.email = builder.email;
    }

    public Range<Integer> getAccountId() {
        return accountId;
    }

    public String getUsername() {
        return username;
    }

    public String getPassword() {
        return password;
    }

    public String getEmail() {
        return email;
    }

    public static class Builder {
        private Range<Integer> accountId;
        private String username;
        private String password;
        private String email;

        public Builder() {
            this.accountId = null;
            this.username = null;
            this.password = null;
            this.email = null;
        }

        public Builder accountId(final int accountId) {
            this.accountId = new Range<>(accountId);
            return this;
        }

        public Builder accountId(final Range<Integer> accountId) {
            this.accountId = accountId;
            return this;
        }

        public Builder username(final String username) {
            this.username = username;
            return this;
        }

        public Builder password(final String password) {
            this.password = password;
            return this;
        }

        public Builder email(final String email) {
            this.email = email;
            return this;
        }

        public AccountConstraint build() {
            return new AccountConstraint(this);
        }
    }
}

An extra nice feature now is that the fields that are not-mandatory do not need to be filled in anymore as the default value is null for them anyways.

Upvotes: 0

Views: 957

Answers (2)

fge
fge

Reputation: 121740

One possibility is to use a builder, like so (code to be improved):

public final class AccountConstraintBuilder
{
    private Range<Integer> accountId;
    private String username;
    private String password;
    private String email;

    public AccountConstraintsBuilder()
    {
    }

    public AccountConstraintsBuilder withUsername(final String username)
    {
        this.username = username;
        return this;
    }

    // password, email...

    public AccountConstraintsBuilder withUniqueValue(final int value)
    {
        return withRange(value, value);    
    }

    public AccountConstraintsBuilder withRange(final int min, final int max)
    {
        accountId = new Range<Integer>(min, max);
        return this;
    }

    public AccountConstraint build()
    {
        return new AccountConstraint(accountId, username, password, email);
    }
}

This would then allow to have only one constructor on AccountConstraint: the one with a Range as an argument. Also, builders are a great place to enforce constraints: your instance constructor has nothing to do but "blindly" accept its arguments.

Note: I guess instance parameters in your AccountConstraint could be final, too.

Note2: this could be a static inner class to AccountConstraint instead; and a static factory method could be created so that you could do:

AccountConstraint.newBuilder().withUsername().etc().etc().build();

Upvotes: 1

John B
John B

Reputation: 32959

Neither, use overloading...

public AccountConstraint(Integer accountId, final String username, 
         final String password, final String email) {
    this(new Range<Integer>(accountId), username, password, email);
}

public AccountConstraint(Range<Integer> accountId, final String username, 
         final String password, final String email) {
    this.accountId = accountId;
    this.username = username;
    this.password = password;
    this.email = email;
}

Upvotes: 6

Related Questions