dsaff / truth-old

Truth: we've made Failure a Strategy
Apache License 2.0
5 stars 2 forks source link

is(T)? #17

Open dsaff opened 13 years ago

dsaff commented 13 years ago

In short, should we try to make sure that ASSERT.that(9).is("a") will not compile?

cgruber commented 13 years ago

I want that. It's where I was hoping we take it.

dsaff commented 13 years ago

OK, looking on where the code has gotten to, I'm still not sold. A fair amount of code complexity is coming from generics, and half the complexity from generics is coming from supporting this one requirement.

But, at best, this can only give us some of the help we might hope for. For example, right now, this doesn't compile:

ASSERT.that(3).is(new Date());

But this does:

ASSERT.that(new Date()).is(3);

I think that getting people used to the idea that this is a compile-time safety net will lead to problems. For example, what if someone types (as they often do):

ASSERT.that(myServer.getUrl()).is("http://www.google.com");

This compiles, and returns "Not true that http://www.google.com is http://www.google.com"

This will cause a lot of head-scratching: it compiles, so the types must be right, so where is this error coming from? Only much later might someone realize that getUrl() returns a URL.

I'd rather people not depend on any compile-time help here, and instead give them a better error message:

Not true that URLhttp://www.google.com is Stringhttp://www.google.com

The only thing lost if someone happens to get the types wrong in a way this could have caught is that they run it one extra time, get the failure, and fix the test. It's true that we could go ahead and give them both a half-helpful compile-time check, and better error messages. But I still am not convinced this complexity is paying for itself, especially when I look at CollectionSubject, et al.

cgruber commented 13 years ago

I'm a bit confused. I am not sure that ASSERT.that(myServer.getUrl()).is("http://www.google.com"); should compile. I think we want is() to be a stronger type than Object to avoid exactly this.

Christian.

On Aug 12, 2011, at 4:25 PM, dsaff wrote:

OK, looking on where the code has gotten to, I'm still not sold. A fair amount of code complexity is coming from generics, and half the complexity from generics is coming from supporting this one requirement.

But, at best, this can only give us some of the help we might hope for. For example, right now, this doesn't compile:

ASSERT.that(3).is(new Date());

But this does:

ASSERT.that(new Date()).is(3);

I think that getting people used to the idea that this is a compile-time safety net will lead to problems. For example, what if someone types (as they often do):

ASSERT.that(myServer.getUrl()).is("http://www.google.com");

This compiles, and returns "Not true that http://www.google.com is http://www.google.com"

This will cause a lot of head-scratching: it compiles, so the types must be right, so where is this error coming from? Only much later might someone realize that getUrl() returns a URL.

I'd rather people not depend on any compile-time help here, and instead give them a better error message:

Not true that URLhttp://www.google.com is Stringhttp://www.google.com

The only thing lost if someone happens to get the types wrong in a way this could have caught is that they run it one extra time, get the failure, and fix the test. It's true that we could go ahead and give them both a half-helpful compile-time check, and better error messages. But I still am not convinced this complexity is paying for itself, especially when I look at CollectionSubject, et al.

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1794515

dsaff commented 13 years ago

So you're suggesting that public DefaultSubject that(Object target) should be DefaultSubject that (T target), or did you have another solution in mind?

David Saff

On Fri, Aug 12, 2011 at 5:06 PM, cgruber reply@reply.github.com wrote:

I'm a bit confused.  I am not sure that ASSERT.that(myServer.getUrl()).is("http://www.google.com"); should compile.  I think we want is() to be a stronger type than Object to avoid exactly this.

Christian.

On Aug 12, 2011, at 4:25 PM, dsaff wrote:

OK, looking on where the code has gotten to, I'm still not sold.  A fair amount of code complexity is coming from generics, and half the complexity from generics is coming from supporting this one requirement.

But, at best, this can only give us some of the help we might hope for.  For example, right now, this doesn't compile:

ASSERT.that(3).is(new Date());

But this does:

ASSERT.that(new Date()).is(3);

I think that getting people used to the idea that this is a compile-time safety net will lead to problems.  For example, what if someone types (as they often do):

ASSERT.that(myServer.getUrl()).is("http://www.google.com");

This compiles, and returns "Not true that http://www.google.com is http://www.google.com"

This will cause a lot of head-scratching: it compiles, so the types must be right, so where is this error coming from?  Only much later might someone realize that getUrl() returns a URL.

I'd rather people not depend on any compile-time help here, and instead give them a better error message:

Not true that URLhttp://www.google.com is Stringhttp://www.google.com

The only thing lost if someone happens to get the types wrong in a way this could have caught is that they run it one extra time, get the failure, and fix the test.  It's true that we could go ahead and give them both a half-helpful compile-time check, and better error messages.  But I still am not convinced this complexity is paying for itself, especially when I look at CollectionSubject, et al.

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1794515

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1794802

cgruber commented 13 years ago

WAit... no... Maybe I'm misunderstanding the problem.

Oh... I see it now. The below code matches on that(Object) and is(Object). Hmm. I see. I was completely wrong, sorry.

At some level, this is a problem of URL's toString() sucking, frankly. I'm not sure how we solve that, unless in DefaultObject's is, we explicitly change the output text for failure to include the concrete type. Something like

"Not true that (URL) http://www.google.com is (String) http://www.google.com"

I think we can use Class.simpleName() for this visual disambiguation.

Christian.

On Aug 15, 2011, at 7:25 AM, dsaff wrote:

So you're suggesting that public DefaultSubject that(Object target) should be DefaultSubject that (T target), or did you have another solution in mind?

David Saff

On Fri, Aug 12, 2011 at 5:06 PM, cgruber reply@reply.github.com wrote:

I'm a bit confused. I am not sure that ASSERT.that(myServer.getUrl()).is("http://www.google.com"); should compile. I think we want is() to be a stronger type than Object to avoid exactly this.

Christian.

On Aug 12, 2011, at 4:25 PM, dsaff wrote:

OK, looking on where the code has gotten to, I'm still not sold. A fair amount of code complexity is coming from generics, and half the complexity from generics is coming from supporting this one requirement.

But, at best, this can only give us some of the help we might hope for. For example, right now, this doesn't compile:

ASSERT.that(3).is(new Date());

But this does:

ASSERT.that(new Date()).is(3);

I think that getting people used to the idea that this is a compile-time safety net will lead to problems. For example, what if someone types (as they often do):

ASSERT.that(myServer.getUrl()).is("http://www.google.com");

This compiles, and returns "Not true that http://www.google.com is http://www.google.com"

This will cause a lot of head-scratching: it compiles, so the types must be right, so where is this error coming from? Only much later might someone realize that getUrl() returns a URL.

I'd rather people not depend on any compile-time help here, and instead give them a better error message:

Not true that URLhttp://www.google.com is Stringhttp://www.google.com

The only thing lost if someone happens to get the types wrong in a way this could have caught is that they run it one extra time, get the failure, and fix the test. It's true that we could go ahead and give them both a half-helpful compile-time check, and better error messages. But I still am not convinced this complexity is paying for itself, especially when I look at CollectionSubject, et al.

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1794515

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1794802

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1807465

dsaff commented 13 years ago

So you now believe ASSERT.that(myServer.getUrl()).is("http://www.google.com") should compile, or that we should introduce changes so that it won't?

cgruber commented 13 years ago

I'm not sure how we can make it not compile. If you want to get to "object" as the type compared, then this should compile. Unless we decide that is() shouldn't exist for the default object, to avoid such bad comparisons. I could go either way (for DefaultSubject) as long as we make the failure more clear to the reader.

Regards, Christian Sent from my iPhone.

On Aug 15, 2011, at 13:27, dsaff reply@reply.github.com wrote:

So you now believe ASSERT.that(myServer.getUrl()).is("http://www.google.com") should compile, or that we should introduce changes so that it won't?

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1810592

dsaff commented 13 years ago

I agree that our output can be better, and that we should look for ways to do that. That's somewhat orthogonal, however, to what we want to compile

So there are many places we could go with this:

1) This is the current situation:

Consequences:

2) This is a proposal that I raised, and I thought at one point you preferred, but now I'm not sure:

Consequences:

3) This is my favorite proposal:

Consequences:

Why do I prefer #3?

What's your preference, and why?

David Saff

On Mon, Aug 15, 2011 at 8:51 PM, cgruber reply@reply.github.com wrote:

I'm not sure how we can make it not compile.  If you want to get to "object" as the type compared, then this should compile.  Unless we decide that is() shouldn't exist for the default object, to avoid such bad comparisons.  I could go either way (for DefaultSubject) as long as we make the failure more clear to the reader.

Regards, Christian Sent from my iPhone.

On Aug 15, 2011, at 13:27, dsaff reply@reply.github.com wrote:

So you now believe ASSERT.that(myServer.getUrl()).is("http://www.google.com") should compile, or that we should introduce changes so that it won't?

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1810592

Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth-old/issues/17#issuecomment-1812607