-
Notifications
You must be signed in to change notification settings - Fork 2
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
Should Object-based methods be in the base Subject? #20
Comments
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()). |
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. |
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:
|
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. |
Oh, I was assuming StringSubject would inherit from ObjectSubject... On Mon, Jun 13, 2011 at 2:16 PM, 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:
|
OK, I think names are beginning to get in our way here. I'm imagining: current world: Subject { nextChain, fail, isA, is, ... } new, proposed world: Subject { nextChain, fail, ... } Does this help? On Mon, Jun 13, 2011 at 2:39 PM, 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:
|
What is a situation where someone would really want to 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
|
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:
|
On Tue, Jun 14, 2011 at 1:33 PM, cgruber
This will trigger ASSERT.that(Object), which will return ObjectSubject.
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
|
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:
|
I'm not as worried about whether Java lets you do stupid things (it So, for all primitive types xxx, I'd like to prevent (assuming "is" is (xxx).isA I'd also like to prevent (double).isEqualTo(double) Again, where xxx is any primitive, I'm not a big fan of (xxx).isNull But I can cope with either decision. Note that the proposed separation of Subject and ObjectSubject also ASSERT.with(USERS).that(currentUser()).isLoggedIn() Without exposing the ability to say ASSERT.with(USERS).that(currentUser()).isA(ImplementationSpecificInstanceOfSomeUserClass.class) Thoughts? David Saff
|
Arggh. Please s/[.]with/.about/ throughout. David Saff On Tue, Jun 14, 2011 at 2:41 PM, David Saff david@saff.net wrote:
|
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. |
Sounds good. Am I blocking the truth0 move? Can I help? |
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. |
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.
The text was updated successfully, but these errors were encountered: