Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEST-478: Add support for parametrized types in AbstractAssert #11

Merged
merged 1 commit into from
Apr 29, 2012
Merged

Conversation

ash2k
Copy link
Contributor

@ash2k ash2k commented Apr 22, 2012

@joel-costigliola
Copy link
Contributor

Thanks I hope to have time to review it this week

@joel-costigliola
Copy link
Contributor

Hi Mikhail,

I have made some modifications to your commit, in order to constrain the selfType in AbstractAssert constructor, it is now : Class selfType instead of Class. I think this is better as it constrains selfType.
It has impacted your unit tests as cast is now needed.

You can review this in this branch or directly in this commit.

Tell me what do you think and if it still fulfill your initial demand.

Cheers,

Joel

@ash2k
Copy link
Contributor Author

ash2k commented Apr 29, 2012

Hi Joel,

I looked at the code. I had this idea too. I thought this is not a good way of designing an API to make it's users have to deal with such warnings. But here we can't do anything better because of Java's type system. Now i understand that this approach is type-safer even thought it produces warnings. Anyway, it solves my original problem.

Thanks!

joel-costigliola added a commit that referenced this pull request Apr 29, 2012
FEST-478: Add support for parametrized types in AbstractAssert

Mikhail,

Good point, we should offer the easiest API to our users, so letting them dealing with generics warning is not a good idea.
I thus prefer we deal with lesser type checking since it is mostly internal (I said mostly because when extending assertions the constructor now accept any class :

```java
  protected AbstractAssert(A actual, Class<?> selfType) {
    myself = (S) selfType.cast(this);
    this.actual = actual;
    info = new WritableAssertionInfo();
  }
```

Anyway, when I use a third party library and I must do some cast due to generics I always feel bad, because I wonder if I did use the library the wrong way or if the library force me to cast.

So let's merge your work.
@joel-costigliola joel-costigliola merged commit 2d6f584 into alexruiz:master Apr 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants