Reputation: 69279
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
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
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