Should Object-based methods be in the base Subject?

[dsaff]

Currently, the Subject base class contains Object-based methods: is, isEqualTo, isInstanceOf, etc...

I think there's an advantage to pushing those methods down to an ObjectSubject class. For example, there could be subjects for which isEqualTo isn't defined/useful. We may choose to do this with IntSubject: ASSERT.that(x).is(373) is of questionable utility. To leave this option open, and due to SRP, I think this might be a good idea. DefaultSubject may not need to be anything better than ObjectSubject, in this situation.

[cgruber]

We'll have problems with chaining if we do that, since we'll have something match Object to get isInstanceOf and then can't do any non-Object work.

Now is, isEqualTo may be reasonable, so the custom types need to do their own, but instanceof (isA()) is appropriate for the base, I think, since they all need it.

Let's draw up a table of things that are truly universal (should be on Subject) and things that are specific, but for which we need Object implementations for default behavour (isEqualTo()).

[dsaff]

Can you give an example of a chain that works now, but wouldn't in my suggested world? I think I've swapped an important point out of memory. Thanks.

[cgruber]

Wouldn't ASSERT.that("some string").isA(String.class).and().contains("some");

The Subject returned from isA() would be DefaultSubject (or ObjectSubject or whatever), which doesn't contain any of StringSubject's contract.

Christian.

On Jun 13, 2011, at 11:01 AM, dsaff wrote:

Can you give an example of a chain that works now, but wouldn't in my suggested world? I think I've swapped an important point out of memory. Thanks.

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

[cgruber]

Actually, it wouldn't compile, and furthermore, you couldn't get isA() on ASSERT.that("some string"). That would return StringSubject, which would not have the isA() contract in this universe.

[dsaff]

Oh, I was assuming StringSubject would inherit from ObjectSubject...

On Mon, Jun 13, 2011 at 2:16 PM, cgruber reply@reply.github.com wrote:

Actually, it wouldn't compile, and furthermore, you couldn't get isA() on ASSERT.that("some string").  That would return StringSubject, which would not have the isA() contract in this universe.

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

[cgruber]

Dude... "Subject" is "ObjectSubject". That's how it works now. I just have "DefaultSubject so that things that don't match have a concrete thing to match on. It's a no-op sub-class of Subject. But Subject does contain all the isA() stuff.

On Jun 13, 2011, at 11:36 AM, dsaff wrote:

Oh, I was assuming StringSubject would inherit from ObjectSubject...

On Mon, Jun 13, 2011 at 2:16 PM, cgruber reply@reply.github.com wrote:

Actually, it wouldn't compile, and furthermore, you couldn't get isA() on ASSERT.that("some string"). That would return StringSubject, which would not have the isA() contract in this universe.

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

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

[dsaff]

OK, I think names are beginning to get in our way here. I'm imagining:

current world:

Subject { nextChain, fail, isA, is, ... } DefaultSubject extends Subject {} StringSubject extends Subject { contains, startsWith, ... } IntegerSubject extends Subject { isBetween, @Override isEqualTo, ... }

new, proposed world:

Subject { nextChain, fail, ... } ObjectSubject extends Subject { isA, is, ... } StringSubject extends ObjectSubject { contains, startsWith, ... } IntegerSubject extends Subject { isBetween, isEqualTo }

Does this help?

On Mon, Jun 13, 2011 at 2:39 PM, cgruber reply@reply.github.com wrote:

Dude... "Subject" is "ObjectSubject".  That's how it works now.  I just have "DefaultSubject so that things that don't match have a concrete thing to match on.  It's a no-op sub-class of Subject.  But Subject does contain all the isA() stuff.

On Jun 13, 2011, at 11:36 AM, dsaff wrote:

Oh, I was assuming StringSubject would inherit from ObjectSubject...

On Mon, Jun 13, 2011 at 2:16 PM, cgruber reply@reply.github.com wrote:

Actually, it wouldn't compile, and furthermore, you couldn't get isA() on ASSERT.that("some string").  That would return StringSubject, which would not have the isA() contract in this universe.

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

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

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

[cgruber]

Ahhhhh... sorry I see it now. You're adding a layer between Subject and StringSubject which would NOT be a parent to IntegerSubject?

The problem is that isA is applicable to primitive types, etc. I think there's less contract delta between Subject and ObjectSubject than you might think. I can see isA() and is() being applicable to primitives... what other contract would you imagine being different?

Christian

On Jun 14, 2011, at 5:36 AM, dsaff wrote:

OK, I think names are beginning to get in our way here. I'm imagining:

current world:

Subject { nextChain, fail, isA, is, ... } DefaultSubject extends Subject {} StringSubject extends Subject { contains, startsWith, ... } IntegerSubject extends Subject { isBetween, @Override isEqualTo, ... }

new, proposed world:

Subject { nextChain, fail, ... } ObjectSubject extends Subject { isA, is, ... } StringSubject extends ObjectSubject { contains, startsWith, ... } IntegerSubject extends Subject { isBetween, isEqualTo }

Does this help?

On Mon, Jun 13, 2011 at 2:39 PM, cgruber reply@reply.github.com wrote:

Dude... "Subject" is "ObjectSubject". That's how it works now. I just have "DefaultSubject so that things that don't match have a concrete thing to match on. It's a no-op sub-class of Subject. But Subject does contain all the isA() stuff.

On Jun 13, 2011, at 11:36 AM, dsaff wrote:

Oh, I was assuming StringSubject would inherit from ObjectSubject...

On Mon, Jun 13, 2011 at 2:16 PM, cgruber reply@reply.github.com wrote:

Actually, it wouldn't compile, and furthermore, you couldn't get isA() on ASSERT.that("some string"). That would return StringSubject, which would not have the isA() contract in this universe.

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

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

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

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

[dsaff]

What is a situation where someone would really want to ASSERT.that(3.0).isA(double.class)?

A big thing I want to just remove from being possible is

ASSERT.that(3.00000001).isEqualTo(3.000000002);

David Saff

On Tue, Jun 14, 2011 at 12:22 PM, cgruber reply@reply.github.com wrote:

Ahhhhh... sorry I see it now.  You're adding a layer between Subject and StringSubject which would NOT be a parent to IntegerSubject?

The problem is that isA is applicable to primitive types, etc.  I think there's less contract delta between Subject and ObjectSubject than you might think.  I can see isA() and is() being applicable to primitives... what other contract would you imagine being different?

Christian

On Jun 14, 2011, at 5:36 AM, dsaff wrote:

OK, I think names are beginning to get in our way here.  I'm imagining:

current world:

Subject { nextChain, fail, isA, is, ... } DefaultSubject extends Subject {} StringSubject extends Subject { contains, startsWith, ... } IntegerSubject extends Subject { isBetween, @Override isEqualTo, ... }

new, proposed world:

Subject { nextChain, fail, ... } ObjectSubject extends Subject { isA, is, ... } StringSubject extends ObjectSubject { contains, startsWith, ... } IntegerSubject extends Subject { isBetween, isEqualTo }

Does this help?

On Mon, Jun 13, 2011 at 2:39 PM, cgruber reply@reply.github.com wrote:

Dude... "Subject" is "ObjectSubject".  That's how it works now.  I just have "DefaultSubject so that things that don't match have a concrete thing to match on.  It's a no-op sub-class of Subject.  But Subject does contain all the isA() stuff.

On Jun 13, 2011, at 11:36 AM, dsaff wrote:

Oh, I was assuming StringSubject would inherit from ObjectSubject...

On Mon, Jun 13, 2011 at 2:16 PM, cgruber reply@reply.github.com wrote:

Actually, it wouldn't compile, and furthermore, you couldn't get isA() on ASSERT.that("some string").  That would return StringSubject, which would not have the isA() contract in this universe.

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

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

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

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

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

[cgruber]

ASSERT.that(someObjectVariableWhichMayContainANumeric).isA(Double.class);

And why do you want to remove ASSERT.that(3.00000001).isEqualTo(3.000000002); from being possible?

On Jun 14, 2011, at 10:24 AM, dsaff wrote:

What is a situation where someone would really want to ASSERT.that(3.0).isA(double.class)?

A big thing I want to just remove from being possible is

ASSERT.that(3.00000001).isEqualTo(3.000000002);

David Saff

[dsaff]

On Tue, Jun 14, 2011 at 1:33 PM, cgruber reply@reply.github.com wrote:

ASSERT.that(someObjectVariableWhichMayContainANumeric).isA(Double.class);

This will trigger ASSERT.that(Object), which will return ObjectSubject.

And why do you want to remove ASSERT.that(3.00000001).isEqualTo(3.000000002); from being possible?

Because people always

ASSERT.that(2.0 + 2.0).isEqualTo(4.0),

and get flustered when it fails. There must be a delta, or it isn't meaningful.

David

On Jun 14, 2011, at 10:24 AM, dsaff wrote:

What is a situation where someone would really want to ASSERT.that(3.0).isA(double.class)?

A big thing I want to just remove from being possible is

ASSERT.that(3.00000001).isEqualTo(3.000000002);

  David Saff

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

[cgruber]

So, I think doubles are a separate issue, and I think you're trying to use the contract heirarchy to deal with the fact that Java lets you do stupid things. Maybe that makes sense. But IntegerSubject can reasonably do .that(4).is(4). It's just floating point and other inexact numerics. I wonder if we should fix it by not allowing is() for that one, or if it's more sensible to have errors from the DoubleSubject which are more robust.

The other possibility is the separation you suggest, though I wouldn't make all primitives inherit from the root - rather, than ObjectSubject, make it EquatableSubject or something. Because I think booleans and Integers and just about everything except doubles/floats should be equatable.

Christian.

On Jun 14, 2011, at 10:46 AM, dsaff wrote:

On Tue, Jun 14, 2011 at 1:33 PM, cgruber reply@reply.github.com wrote:

ASSERT.that(someObjectVariableWhichMayContainANumeric).isA(Double.class);

This will trigger ASSERT.that(Object), which will return ObjectSubject.

And why do you want to remove ASSERT.that(3.00000001).isEqualTo(3.000000002); from being possible?

Because people always

ASSERT.that(2.0 + 2.0).isEqualTo(4.0),

and get flustered when it fails. There must be a delta, or it isn't meaningful.

David

On Jun 14, 2011, at 10:24 AM, dsaff wrote:

What is a situation where someone would really want to ASSERT.that(3.0).isA(double.class)?

A big thing I want to just remove from being possible is

ASSERT.that(3.00000001).isEqualTo(3.000000002);

David Saff

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

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

[dsaff]

I'm not as worried about whether Java lets you do stupid things (it does) than whether a user scanning the API will see a method that seems to do a smart thing, and instead does something stupid.

So, for all primitive types xxx, I'd like to prevent (assuming "is" is an alias, uninteresting to this conversation):

(xxx).isA (xxx).isNotA (xxx).isSameAs

I'd also like to prevent

(double).isEqualTo(double) (double).isNotEqualTo(double)

Again, where xxx is any primitive, I'm not a big fan of

(xxx).isNull (xxx).isNotNull

But I can cope with either decision.

Note that the proposed separation of Subject and ObjectSubject also allows for custom subjects whose authors would like to hide information about the wrapped objects, to encourage robust test authoring. For example, I might want to enable:

ASSERT.with(USERS).that(currentUser()).isLoggedIn()

Without exposing the ability to say

ASSERT.with(USERS).that(currentUser()).isA(ImplementationSpecificInstanceOfSomeUserClass.class)

Thoughts?

David Saff On Tue, Jun 14, 2011 at 1:54 PM, cgruber reply@reply.github.com wrote:

So, I think doubles are a separate issue, and I think you're trying to use the contract heirarchy to deal with the fact that Java lets you do stupid things. Maybe that makes sense.  But IntegerSubject can reasonably do .that(4).is(4).  It's just floating point and other inexact numerics.  I wonder if we should fix it by not allowing is() for that one, or if it's more sensible to have errors from the DoubleSubject which are more robust.

The other possibility is the separation you suggest, though I wouldn't make all primitives inherit from the root - rather, than ObjectSubject, make it EquatableSubject or something.  Because I think booleans and Integers and just about everything except doubles/floats should be equatable.

Christian.

On Jun 14, 2011, at 10:46 AM, dsaff wrote:

On Tue, Jun 14, 2011 at 1:33 PM, cgruber reply@reply.github.com wrote:

ASSERT.that(someObjectVariableWhichMayContainANumeric).isA(Double.class);

This will trigger ASSERT.that(Object), which will return ObjectSubject.

And why do you want to remove ASSERT.that(3.00000001).isEqualTo(3.000000002); from being possible?

Because people always

ASSERT.that(2.0 + 2.0).isEqualTo(4.0),

and get flustered when it fails.  There must be a delta, or it isn't meaningful.

  David

On Jun 14, 2011, at 10:24 AM, dsaff wrote:

What is a situation where someone would really want to ASSERT.that(3.0).isA(double.class)?

A big thing I want to just remove from being possible is

ASSERT.that(3.00000001).isEqualTo(3.000000002);

  David Saff

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

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

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

[dsaff]

Arggh. Please s/[.]with/.about/ throughout.

David Saff

On Tue, Jun 14, 2011 at 2:41 PM, David Saff david@saff.net wrote:

I'm not as worried about whether Java lets you do stupid things (it does) than whether a user scanning the API will see a method that seems to do a smart thing, and instead does something stupid.

So, for all primitive types xxx, I'd like to prevent (assuming "is" is an alias, uninteresting to this conversation):

(xxx).isA (xxx).isNotA (xxx).isSameAs

I'd also like to prevent

(double).isEqualTo(double) (double).isNotEqualTo(double)

Again, where xxx is any primitive, I'm not a big fan of

(xxx).isNull (xxx).isNotNull

But I can cope with either decision.

Note that the proposed separation of Subject and ObjectSubject also allows for custom subjects whose authors would like to hide information about the wrapped objects, to encourage robust test authoring.  For example, I might want to enable:

ASSERT.with(USERS).that(currentUser()).isLoggedIn()

Without exposing the ability to say

ASSERT.with(USERS).that(currentUser()).isA(ImplementationSpecificInstanceOfSomeUserClass.class)

Thoughts?

  David Saff On Tue, Jun 14, 2011 at 1:54 PM, cgruber reply@reply.github.com wrote:

So, I think doubles are a separate issue, and I think you're trying to use the contract heirarchy to deal with the fact that Java lets you do stupid things. Maybe that makes sense.  But IntegerSubject can reasonably do .that(4).is(4).  It's just floating point and other inexact numerics.  I wonder if we should fix it by not allowing is() for that one, or if it's more sensible to have errors from the DoubleSubject which are more robust.

The other possibility is the separation you suggest, though I wouldn't make all primitives inherit from the root - rather, than ObjectSubject, make it EquatableSubject or something.  Because I think booleans and Integers and just about everything except doubles/floats should be equatable.

Christian.

On Jun 14, 2011, at 10:46 AM, dsaff wrote:

On Tue, Jun 14, 2011 at 1:33 PM, cgruber reply@reply.github.com wrote:

ASSERT.that(someObjectVariableWhichMayContainANumeric).isA(Double.class);

This will trigger ASSERT.that(Object), which will return ObjectSubject.

And why do you want to remove ASSERT.that(3.00000001).isEqualTo(3.000000002); from being possible?

Because people always

ASSERT.that(2.0 + 2.0).isEqualTo(4.0),

and get flustered when it fails.  There must be a delta, or it isn't meaningful.

  David

On Jun 14, 2011, at 10:24 AM, dsaff wrote:

What is a situation where someone would really want to ASSERT.that(3.0).isA(double.class)?

A big thing I want to just remove from being possible is

ASSERT.that(3.00000001).isEqualTo(3.000000002);

  David Saff

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

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

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

[cgruber]

Ok. I think, after re-reading this, I can jibe with this. I'd like to migrate over to truth0/truth first, so if we can get David's work in and do that, then we can move on this stuff.

[dsaff]

Sounds good. Am I blocking the truth0 move? Can I help?

[cgruber]

Just pulling in hagbard's changes when they're ready. There's an outstanding pull request I'd rather have in because it fixes the build.