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

Relation between two values in the constructor #747

Closed
mazurkin opened this issue Dec 21, 2022 · 8 comments
Closed

Relation between two values in the constructor #747

mazurkin opened this issue Dec 21, 2022 · 8 comments

Comments

@mazurkin
Copy link

Simple test of the simple record

    record MyRecord(int total, int used) {

        MyRecord(int total, int used) {
            Preconditions.checkArgument(total >= 0);
            this.total = total;

            Preconditions.checkArgument(used >= 0);
            Preconditions.checkArgument(used <= total);
            this.used = used;
        }
    }

    @Test
    public void testMyRecord() {
        EqualsVerifier
            .forClass(MyRecord.class)
            .verify();
    }
java.lang.AssertionError: EqualsVerifier found a problem in class Test$MyRecord.
-> Record: failed to run constructor for record type Test$MyRecord
  These were the values passed to the constructor: [0, 1]
  If the record does not accept 0 or false as a value for its constructor parameters, consider suppressing Warning.ZERO_FIELDS.
  If the record does not accept any of the given values for its constructor parameters, consider providing prefab values for the types of those fields.

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages
(EqualsVerifier 3.12.3, JDK 17.0.5 on Linux)

Supression suppress(Warning.ZERO_FIELDS) doesn't help as it fails with:

These were the values passed to the constructor: [1, 2]

Probably worth to create withPrefabValues() which takes a name of the constructor's field.

@mazurkin
Copy link
Author

also more complicated bean:

  record MyRecord(int total, Collection<String> samples) {

        MyRecord(int total, int used) {
            Preconditions.checkArgument(total >= 0);
            this.total = total;

            Preconditions.checkNonNull(samples)
            Preconditions.checkArgument(samples.size() <= total);
            this.samples = samples;
        }
    }

@jqno
Copy link
Owner

jqno commented Dec 23, 2022

Thanks for reporting this, this is indeed a problem that I don't have a solution for at the moment...

Your suggestion for adding a withPrefabValues overload which also takes the names of the fields is interesting, but I'm not sure if it's the best solution...I'll have to think about this a little.

@ErrorProne
Copy link

Hey hey there, any news on this?
I've a similiar use-case, nowadays I use records a lot as "custom types" for properties which have special requirements (must be all upper case, follow a specific pattern etc.) and the constructor enforces those requirements. So something like @mazurkin mentioned.
Which results in a different error right now, but its pretty much the same reason I guess

java.lang.AssertionError: EqualsVerifier found a problem in class ExampleTest$Example.
-> Significant fields: equals does not use value, or it is stateless.

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages
(EqualsVerifier 3.16.2, JDK 21.0.3 running on classpath, on Mac OS X)

@jqno
Copy link
Owner

jqno commented Aug 24, 2024

Sorry for the long radio silence on this. However, this issue has been on my mind. I agree with @mazurkin that adding an overload to withPrefabValues is probably the best solution. However, this requires some re-architecting in EqualsVerifier's internals. I've been trying to get to it, but life kept getting in the way. I hope to start working on it soon, though.

@ErrorProne
Copy link

Sorry for the long radio silence on this. However, this issue has been on my mind. I agree with @mazurkin that adding an overload to withPrefabValues is probably the best solution. However, this requires some re-architecting in EqualsVerifier's internals. I've been trying to get to it, but life kept getting in the way. I hope to start working on it soon, though.

I can totally see that. I'm happy to help if possible, but I would totally understand if you want to do such a change on your lib by yourself.

@jqno
Copy link
Owner

jqno commented Aug 26, 2024

Thanks for the offer! Normally I would totally take you up on that, but indeed in this case it might not be practical, unfortunately

This was referenced Sep 19, 2024
@jqno
Copy link
Owner

jqno commented Sep 24, 2024

Believe it or not, I have a solution for this issue. I've just released version 3.17, which adds a method 'withPrefabValuesForField()` that you can use to come up with prefab values for a specific field, and that field alone. This will make it possible to enforce invariants like these.

You have to make sure when using multiple parameters that depend on each other, that the first ones work together, and the second ones do too. So for the OP's example, that would look like:

EqualsVerifier
    .forClass(MyRecord.class)
    .withPrefabValuesForField("total", 10, 20)
    .withPrefabValuesForField("used", 8, 18)
    .verify();

As you can see, 10 and 8 go together, as do 20 and 18.

I'm planning on adding another feature soon-ish, which should make this even easier, but for now this should work.

@jqno jqno closed this as completed Sep 24, 2024
@ErrorProne
Copy link

Works perfectly! Thank you very much

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

No branches or pull requests

3 participants