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

Validation class to avoid ugly if-else blocks

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

Problem

I've answered this question on Stack Overflow. The problem is - there are 30 columns in an Excel sheet and I need to validate each of the fields with different validation logic. The code should avoid a lot of if-else blocks. I have used strategy pattern to solve this. I would like to get a review of this code.

interface IValidator {
  boolean validate(T field);
}

class SomeFieldOne implements IValidator {
  public boolean validate(T field) {
    System.out.println("SomeFieldOne validation");
    return true; // return true/false based on validation
  }
}

class SomeFieldTwo implements IValidator {
  public boolean validate(T field) {
    System.out.println("SomeFieldTwo validate");
    return true; // return true/false based on validation
  }
}

class Context {
  private IValidator validator;

  public Context(IValidator validator) {
    this.validator = validator;
  }

  public boolean validate(String field) {
    return this.validator.validate(field);
  }
}

public class TestValidation {
  public static void main(String[] args) {
    Context context;

    context = new Context(new SomeFieldOne());
    System.out.println(context.validate("some field one"));

    context = new Context(new SomeFieldTwo());
    System.out.println(context.validate("some field two"));

    // test other fields ....
    // .........
  }
}

Solution

First of all, you're not using your generics properly here:

private IValidator validator;


I would change this to

private final IValidator validator;


Because:

  • final because the value does not change, and should not change. The field should be immutable



  • ` to use generics properly. Otherwise you should get a compiler warning saying IValidator is a raw type. References to generic type IValidator should be parameterized



However, I think your
Context is not very useful. I don't see any reason for why you need it. You can accomplish the exact same things by using:

public class TestValidation {
  public static void main(String[] args) {
    IValidator context;
    context = new SomeFieldOne();
    System.out.println(context.validate("some field one"));
    context = new SomeFieldTwo();
    System.out.println(context.validate("some field two"));
    // test other fields ....
    // .........
  }
}


This is less code and has the same effect. And it also looks better and improves readability as changing strategy now calls only one constructor instead of two.

I assume you're compiling with Java 1.6 or above, and therefore your classes that implements the
IValidator interface should mark the validate method with @Override` to comply with the Java coding conventions. (Marking methods with @Override that overrides interface methods was not supported at all in previous Java versions)

@Override
public boolean validate(T field) {

Code Snippets

private IValidator validator;
private final IValidator<String> validator;
public class TestValidation {
  public static void main(String[] args) {
    IValidator<String> context;
    context = new SomeFieldOne();
    System.out.println(context.validate("some field one"));
    context = new SomeFieldTwo();
    System.out.println(context.validate("some field two"));
    // test other fields ....
    // .........
  }
}
@Override
public boolean validate(T field) {

Context

StackExchange Code Review Q#35491, answer score: 10

Revisions (0)

No revisions yet.