Code Monkey home page Code Monkey logo

Comments (19)

ZakTaccardi avatar ZakTaccardi commented on September 22, 2024 4

@rharter yes, something like @IgnoreHashEquals would make for a great AutoValue extension and be an ideal solution to this problem. The current recommended solution is certainly too verbose.

from auto.

PhilippWendler avatar PhilippWendler commented on September 22, 2024 3

May I ask for reconsidering the inclusion of this feature in AutoValue
(regardless of whether as direct inclusion or as a bundled extension, because as a user I won't notice any difference)?

@Memoized has been added to AutoValue, and from my point of view both features are pretty similar. In our (large) project, we certainly have more need of ignoring specific fields in equals() instead of memoizing them, and I would even assume this to be more common in general.

If the decision stays as it is, could you maybe explain why @Memoized was included and this not?

In my case, I have a large hierarchy of data classes, which I would very much like to implement using AutoValue. However, all of these classes share one field that must not be part of equals(). Any solution that requires code to be added to every @AutoValue class is too much effort for me because of the large number of involved classes. I could maybe implement something using the wrapper-class approach, but this doubles the number of involved objects, which would be nice to avoid, and I am not sure whether this can be done in an user-invisible way including builders etc.

Of course I could also use one of the two presented third-party extensions, however, I am skeptical whether I should do so. These extensions need to duplicate the logic for generating hashCode() and equals() that AutoValue has, so I would not benefit from any potential improvements in that regard in AutoValue (e.g., if AutoValue implements #93, would this also work if I use on the extensions?). Furthermore, I don't see any guarantee that the hashCode value is calculated the same way by the code from the extensions than by the code from AutoValue, and I don't know if the extensions could even guarantee this (because this would require a guarantee to keep the calculation constant in the future from AutoValue). While I do not need the guarantee that the hashCode calculation stays constant across AutoValue versions, it would certainly be nice to have the hashCode not needlessly change when extending @AutoValue class A { String s1(); } to @AutoValue class A { String s1(); @IgnoreHashEquals String s2(); }.

Or maybe one could even say that the fact that the extensions need to duplicate code present in AutoValue (for generating hashCode and equals) is a sign that the current extension API is not a good fit for this feature, and implementing it directly in AutoValue or in a bundled extension that can reuse code from AutoValue without duplicating it is worth it? (I would still think that implementing this feature directly in AutoValue requires much less code than if it is in an extension, but of course you could still use an extension.)

from auto.

REggar avatar REggar commented on September 22, 2024 2

@kevinmost Annoyingly I spent some time this week creating an extension for similar reasons to you. I've published it anyway to:
https://github.com/REggar/auto-value-ignore-hash-equals

Perhaps, there's something that can be learnt from both libraries...

from auto.

rharter avatar rharter commented on September 22, 2024

This is currently (in SNAPSHOT) doable via extension. Check out Square's Redacted extension.

Redacted is all about excluding sensitive properties from toString(), but the same sort of thing (or perhaps the same extension with another feature) would work for hashCode() and equals().

from auto.

kevinb9n avatar kevinb9n commented on September 22, 2024

The following will appear in a revised version of the User Guide:

How do I...
... ignore certain properties in equals, etc.?

Suppose your value class has an extra field that shouldn't be included in
equals or hashCode computations. One common reason is that it is a "cached"
or derived value based on other properties. In this case, simply define the
field in your abstract class directly; AutoValue will have nothing to do with
it:

@AutoValue
abstract class DerivedExample {
  static DerivedExample create(String realProperty) {
    return new AutoValue_DerivedExample(realProperty);
  }

  abstract String realProperty();

  private String derivedProperty;

  String derivedProperty() {
    // non-thread-safe example
    if (derivedProperty == null) {
      derivedProperty = realProperty().toLowerCase();
    }
  }
}

On the other hand, if the value is user-specified, not derived, the solution is
slightly more elaborate (but still reasonable):

@AutoValue
abstract class IgnoreExample {
  static IgnoreExample create(String normalProperty, String ignoredProperty) {
    IgnoreExample ie = new AutoValue_IgnoreExample(normalProperty);
    ie.ignoredProperty = ignoredProperty;
    return ie;
  }

  abstract String normalProperty();

  private String ignoredProperty; // sadly, it can't be `final`

  String ignoredProperty() {
    return ignoredProperty;
  }
}

Note that in both cases the field ignored for equals and hashCode is also
ignored by toString; to AutoValue it simply doesn't exist.

from auto.

kevinb9n avatar kevinb9n commented on September 22, 2024

imho, this is simple and natural and preferable to using an extension.

from auto.

jbgi avatar jbgi commented on September 22, 2024

another solution would be to wrap the property inside a class (possibly package private) and use the following implementation of hashCode/equals for that class:

int hashCode() { return 0; }
boolean equals(Object o) { return true; }

and then unwrap the property in a derived accessor.

from auto.

kevinb9n avatar kevinb9n commented on September 22, 2024

@jbgi Notably, implementing those eq/hc methods is no different from what you'd get by putting @autovalue on the new auxiliary class (since it wouldn't have any abstract methods, those are approximately the eq/hc implementations you'll get).

So it turns out basically the same as the examples I illustrated above, but with a separation into 2 classes that I'm not sure really accomplishes anything?

from auto.

jbgi avatar jbgi commented on September 22, 2024

@kevinb9n it address the 'problem' of the field not being final, plus it allows it to be part of the generated toString... but yeah... not much.

from auto.

acorn1010 avatar acorn1010 commented on September 22, 2024

@kevinb9n I believe @jbgi's approach with separation into 2 classes also works for AutoValue.Builders, whereas the IgnoreExample above wouldn't. Unless I'm missing something, there's no way to repopulate these ignored fields when using toBuilder() without the 2 class separation or an extension.

from auto.

eamonnmcmanus avatar eamonnmcmanus commented on September 22, 2024

This should work for builders:

@AutoValue
public abstract class IgnoreExample {
  public abstract String normalProperty();

  private String ignoredProperty; // sadly, it can't be `final`

  public String ignoredProperty() {
    return ignoredProperty;
  }

  public static Builder builder() {
    return new AutoValue_IgnoreExample.Builder();
  }

  public Builder toBuilder() {
    Builder builder = autoToBuilder();
    builder.ignoredProperty = this.ignoredProperty;
    return builder;
  }

  abstract Builder autoToBuilder();

  @AutoValue.Builder
  public abstract static class Builder {
    private String ignoredProperty;

    public abstract Builder normalProperty(String s);

    public Builder ignoredProperty(String s) {
      this.ignoredProperty = s;
      return this;
    }

    public IgnoreExample build() {
      IgnoreExample x = autoBuild();
      x.ignoredProperty = this.ignoredProperty;
      return x;
    }

    abstract IgnoreExample autoBuild();
  }
}

It's a fair amount of noise, but then the wrapper-class approach has its own share of noise too.

The key property being exploited here is that the toBuilder() method in the @AutoValue class and the build() method in the @AutoValue.Builder class are identified based on being abstract and having the right return type. Their names are only conventional, so we can use this dodge of having a package-private abstract method (autoToBuilder() and autoBuild()) and a public method that wraps it.

from auto.

kevinmost avatar kevinmost commented on September 22, 2024

Hi,

I stumbled upon this thread a few days ago and thought that this would be a useful extension to have, and a good way for me to get my feet wet with AutoValue extensions. So I created this project. Lots of inspiration taken from the existing AutoValue extension projects out there.

I definitely need to write more robust tests, but it seems to be working well for me where I needed it. I would like to do more with it in the future (for example, maybe add a @UseForHashCodeEquals annotation so that people can create whitelists instead of blacklists, and maybe a @CustomHashCodeEquals annotation that gets applied to the class itself and takes a String[] of the types to blacklist or whitelist).

I'm currently just building it with Jitpack, which is working well enough for me.

I'd appreciate any contributions or feedback!

from auto.

kevinmost avatar kevinmost commented on September 22, 2024

@REggar haha, our libraries are really similar... I used Kotlin for mine, but aside from language idioms, they're pretty much doing the same thing!

from auto.

eamonnmcmanus avatar eamonnmcmanus commented on September 22, 2024

Well, right now there is a solution for your problem thanks to @kevinmost and @REggar. I agree that it is not as simple or as reliable as a native support within AutoValue itself. But I'm not seeing an actual problem other than bigger generated code. We don't have any plans to change the semantics of the equals and hashCode methods. In #93, we rejected the idea of using xor for Map.Entry classes and what's suggested there is that we would simply refuse to generate a hashCode for them.

The downside of implementing this is that it would make the specification for @AutoValue classes that bit more complicated. It's pretty simple at the moment (though the same can't be said for builders).

You do have a point about @Memoized. Since its extension always applies, @Memoized is always available, so it is effectively part of the core specification.

from auto.

rharter avatar rharter commented on September 22, 2024

While I understand some of the concerns here, this is the type of thing the extensions spec was created for. If you check out auto-value-redacted it implements this exact thing for toString(), so it should be quite easy to do the same for equals() and hashcode().

IMHO, it would be preferable to keep the core of AutoValue as simple as possible and move all optional, customizable functionality to extensions (even builders?). In that regard, it does seem weird to me that things like @Memoized are bundled, as aside from that being a first party extension, there's really no reason to bundle it.

from auto.

eamonnmcmanus avatar eamonnmcmanus commented on September 22, 2024

@PhilippWendler could you say a bit more about the extra field you have with every class? It might help to think about the problem with a more concrete example.

from auto.

kevinmost avatar kevinmost commented on September 22, 2024

It might help us, as extension writers, for the AutoValue API to expose some kind of way to get back what the stock hashCode and stock equals calculations for given fields would be? Right now, my extension recreates the entire hashCode and equals implementations based on how I know AutoValue already does it, but if this changes in the future, I'll have to update my extension.

from auto.

rharter avatar rharter commented on September 22, 2024

@kevinmost I don't know that that's really important. AFAIK there's no current guarantee that the implementation will remain constant between versions, and it shouldn't matter, as that's an implementation detail.

As long as your extension generates a valid hashCode and equals that meets the requirements of the Java APIs, then it shouldn't matter what the implementation is, as implementation isn't guaranteed in any case.

from auto.

PhilippWendler avatar PhilippWendler commented on September 22, 2024

@eamonnmcmanus Sure, though I don't think it will help much in this discussion. I have a tree of classes that are used to represent abstract syntax trees (different operators etc.), and in addition to their typical field each has a field with meta information (file name, line number) that is not relevant when comparing two ASTs for equality.

from auto.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.