alexruiz / fest-assert-2.x

FEST Fluent Assertions 2.x
http://fest.easytesting.org
Apache License 2.0
402 stars 69 forks source link

Document Best Practices For Dealing With Inheritance #137

Open JakeWharton opened 11 years ago

JakeWharton commented 11 years ago

Most (if not all) of the class types for which assertions are provided for have a parent type of Object. In practice, the types for which custom assertions will be written will likely have a much more complex hierarchy of classes. How should we deal with this case?

An example, for clarity:

class Human {
  public String name;
}
class Employee extends Human {
  public String jobTitle;
}
class HumanAssert extends AbstractAssert<HumanAssert, Human> {
  public HumanAssert(Human actual) {
    super(actual, HumanAssert.class);
  }

  public HumanAssert hasName(String name) {
    isNotNull();
    assertThat(actual.name).isEqualTo(name);
    return this;
  }
}

Now, how to craft EmployeeAssert? Thus far I've been doing the following:

class EmployeeAssert extends HumanAssert {
  private final Employee actual;

  public EmployeeAssert(Employee actual) {
    super(actual);
    this.actual = actual;
  }

  public EmployeeAssert hasJobTitle(String jobTitle) {
    isNotNull();
    assertThat(actual.jobTitle).isEqualTo(jobTitle);
    return this;
  }
}

This mostly works and allows chained assertions like:

assertThat(employee).hasJobTitle("CEO").hasName("John Smith");

but does not allow:

assertThat(employee).hasName("John Smith").hasJobTitle("CEO");

since the return type of hasName is HumanAssert rather than EmployeeAssert.

If you extrapolate this case to hierarchies that are three or four classes deep you can quickly see it becomes a mess in forcing your assertions to be written in descending order of the hierarchy if you want to continue chaining.

joel-costigliola commented 11 years ago

Interesting !

I have a simple solution to allow chaining but it's far from ideal, you need to override the inherited assertion like this :

  @Override
  public EmployeeAssert hasName(String name) {
    return (EmployeeAssert) super.hasName(name);
  }

Solving the problem with generics would avoid the need to override assertions but I'm not sure it's feasible.

joel-costigliola commented 11 years ago

Ok found how to solve this with generics !

First generify HumanAssert to allow returning type parameter T in assertions method :

public class HumanAssert<T extends HumanAssert<T>> extends AbstractAssert<HumanAssert<T>, Human> {
  public HumanAssert(Human actual) {
    super(actual, HumanAssert.class);
  }

  public T hasName(String name) {
    isNotNull();
    assertThat(actual.name).isEqualTo(name);
    return (T) this;
  }

}

then change EmployeeAssert defintion from :

class EmployeeAssert extends HumanAssert {

to :

public class EmployeeAssert extends HumanAssert<EmployeeAssert> {

This solves the chaining problem at the cost of ugly code ...

joel-costigliola commented 11 years ago

I have got another potential solution : using fest-assertion-generator.

fest-assertion-generator can generate for you EmployeeAssert and HumanAssert and is able to generate assertions for inherited properties so hasName assertion would generated in both classes. Note that the generated EmployeeAssert won't inherit from HumanAssert in the generated code.

JakeWharton commented 11 years ago

I may have to go with the generator. I don't think your solution supports a more complex test case like this:

public class Example {
  public static void main(String... args) {
    Human h = new Human("Jake");
    assertThat(h).hasName("Jake");

    Employee e = new Employee("Jake", "Engineer");
    assertThat(e).hasJobTitle("Engineer").hasName("Jake");
    assertThat(e).hasName("Jake").hasTitle("Engineer");

    UnitedStatesEmployee u = new UnitedStatesEmployee("Jake", "Engineer", "California");
    assertThat(u).isInState("California").hasJobTitle("Engineer").hasName("Jake");
    assertThat(u).hasJobTitle("Engineer").isInState("California").hasName("Jake");
    assertThat(u).hasJobTitle("Engineer").hasName("Jake").isInState("California");
    assertThat(u).hasName("Jake").isInState("California").hasJobTitle("Engineer");
    assertThat(u).hasName("Jake").hasJobTitle("Engineer").isInState("California");
  }

  @SuppressWarnings("unchecked")
  static HumanAssert assertThat(Human actual) {
    return new HumanAssert(actual, HumanAssert.class);
  }

  @SuppressWarnings("unchecked")
  static EmployeeAssert assertThat(Employee actual) {
    return new EmployeeAssert(actual, EmployeeAssert.class);
  }

  @SuppressWarnings("unchecked")
  static UnitedStatesEmployeeAssert assertThat(UnitedStatesEmployee actual) {
    return new UnitedStatesEmployeeAssert(actual, UnitedStatesEmployeeAssert.class);
  }
}

class Human {
  public final String name;

  public Human(String name) {
    this.name = name;
  }
}

class Employee extends Human {
  public final String jobTitle;

  public Employee(String name, String jobTitle) {
    super(name);
    this.jobTitle = jobTitle;
  }
}

class UnitedStatesEmployee extends Employee {
  public final String state;

  UnitedStatesEmployee(String name, String title, String state) {
    super(name, title);
    this.state = state;
  }
}
JakeWharton commented 11 years ago

I solved it with this:

class AbstractHumanAssert<S extends AbstractHumanAssert<S, A>, A extends Human> extends AbstractAssert<S, A> {
  public AbstractHumanAssert(A actual, Class<S> selfType) {
    super(actual, selfType);
  }

  public S hasName(String name) {
    isNotNull();
    Assertions.assertThat(actual.name).isEqualTo(name);
    return myself;
  }
}

class HumanAssert extends AbstractHumanAssert<HumanAssert, Human> {
  public HumanAssert(Human actual) {
    super(actual, HumanAssert.class);
  }
}

class AbstractEmployeeAssert<S extends AbstractEmployeeAssert<S, A>, A extends Employee> extends
    AbstractHumanAssert<S, A> {
  public AbstractEmployeeAssert(A actual, Class<S> selfType) {
    super(actual, selfType);
  }

  public S hasJobTitle(String title) {
    isNotNull();
    Assertions.assertThat(actual.jobTitle).isEqualTo(title);
    return myself;
  }
}

class EmployeeAssert extends AbstractEmployeeAssert<EmployeeAssert, Employee> {
  public EmployeeAssert(Employee actual) {
    super(actual, EmployeeAssert.class);
  }
}

class AbstractUnitedStatesEmployeeAssert<S extends AbstractUnitedStatesEmployeeAssert<S, A>, A extends UnitedStatesEmployee>
    extends AbstractEmployeeAssert<S, A> {
  public AbstractUnitedStatesEmployeeAssert(A actual, Class<S> selfType) {
    super(actual, selfType);
  }

  public S isInState(String state) {
    isNotNull();
    Assertions.assertThat(actual.state).isEqualTo(state);
    return myself;
  }
}

class UnitedStatesEmployeeAssert
    extends AbstractUnitedStatesEmployeeAssert<UnitedStatesEmployeeAssert, UnitedStatesEmployee> {
  public UnitedStatesEmployeeAssert(UnitedStatesEmployee actual) {
    super(actual, UnitedStatesEmployeeAssert.class);
  }
}

Certainly not ideal, but it works for all permutations listed in the previous comment.

joel-costigliola commented 11 years ago

Using generics, I don't think we can do better than your last solution. I'll document that on the wiki for 2.0 release.

Thanks for reporting this !

wjtk commented 11 years ago

Nice! It's a pity that for example ListAssert is not written in such way. If concrete asserts wouldn't have any additional methods, only abstract would add new ones, that would make possibility to extend assert classes from fest in easy way.

For example if I extend ListAssert I will lose fluent chaining after first method not from my new class. If my class could extend for example AbstractListAssert with all "lists" methods, it would be much easier.

Regards, great lib!

JakeWharton commented 11 years ago

Perhaps switching them should be considered. I used this technique extensively for square/fest-android which has a heavily nested hierarchy of classes.