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

Allow custom toString, hashCode, equals #93

Closed
filiph opened this issue Dec 30, 2016 · 17 comments
Closed

Allow custom toString, hashCode, equals #93

filiph opened this issue Dec 30, 2016 · 17 comments
Assignees

Comments

@filiph
Copy link
Contributor

filiph commented Dec 30, 2016

It looks like toString() is overridden by default. I'd like to have the option to provide my own toString(), and have built_value provide it only when it's not explicitly defined on the abstract class.

The strings generated by built_value contain a lot of newlines and get messy, especially when logging (the number one reason to call toString for me).

@gavindoughtie
Copy link

They're also only good for debugging, not for more practical uses. +1

@davidmorgan davidmorgan changed the title Do not override user's toString() Allow custom toString, hashCode, equals Jan 9, 2017
@davidmorgan
Copy link
Collaborator

Makes sense, and this should apply for equals and hashCode too.

For your specific case though, I wonder if it would be better to improve the generated toString. Another option would be to post-process the output of toString when you want to log it.

If lots of people end up writing their own toString then I'd say the generated code has issues :)

What do you think?

@filiph
Copy link
Contributor Author

filiph commented Jan 11, 2017

should apply for equals and hashCode

Not sure about this, I've never needed this for built values. But if it's easier to do it all at once, sure.

For your specific case though, I wonder if it would be better to improve the generated toString.

FWIW, I think I mostly use toString (of any class, not just builtvalues) for logging. For that, I do things like

class Actor {
  // ..
  String toString() => "Actor<$id-$name>";
}

I then use it like this:

log.info("$actor picked up $item");

It needs to be one-line, short, and immediately recognizable for me as a programmer. I don't think this can be automated (unless you want to introduce some kind of annotation like @includeInToString, but then annotating could take more work for me as a user than implementing toString).

The toString that built value provides is still useful, even when logging (especially for things like errors). Maybe it could be renamed to toLongString or something similar?

@davidmorgan
Copy link
Collaborator

#153 doesn't quite do this -- but maybe it does exactly what you want :) ... by allowing easily customizable toString. Built-in options are now multiline with indentation or single line.

@filiph
Copy link
Contributor Author

filiph commented Mar 23, 2017

Interesting. Does this feature have documentation somewhere? I'm not sure where to customize.

@davidmorgan
Copy link
Collaborator

The PR is not merged yet :) ... customization will be here, you can inject your own BuiltValueToStringHelper:

82d0fb9#diff-83380c0b45d211065789ba69356b1effR104

Example:

82d0fb9#diff-2abe97784de0d4085d6534f85b8d6392R25

@zoechi
Copy link
Contributor

zoechi commented Mar 23, 2017

I was just investigating the dart2js output in https://dart-lang.github.io/dump-info-visualizer
and toString() takes a notable part of the size because string literals can't be minified.
It looks like dart2js canonicalizes string literals, but can't with these from toString, because here they are a part of another string literal.
Generating constants and then referencing them might help.

Somewhat related to #155 (dart2js optimization)

@davidmorgan
Copy link
Collaborator

Thanks! Actually I think my pending PR will help there too, because as you can see the literals now match:

82d0fb9#diff-6f8665194007840a7497ff19de358b44L89

@zoechi
Copy link
Contributor

zoechi commented Mar 23, 2017

Yes, that should work as well.

@eEQK
Copy link

eEQK commented Apr 21, 2021

Could we allow for providing custom == and hashCode? I have a use case in which my class has an id field which is the only one I want to use when comparing instances

  @override
  bool operator ==(Object other) {
    if (identical(other, this)) return true;
    return other is SomeItem && id == other.id;
  }

  @override
  int get hashCode => id.hashCode;

for example in kotlin data classes you can override them freely, and in lombok (code gen for java) you can specify which fields to use when generating equals and hashcode:

you can modify which fields are used (and even specify that the output of various methods is to be used) by marking type members with @EqualsAndHashCode.Include or @EqualsAndHashCode.Exclude. Alternatively, you can specify exactly which fields or methods you wish to be used by marking them with @EqualsAndHashCode.Include and using @EqualsAndHashCode(onlyExplicitlyIncluded = true).

~ https://projectlombok.org/features/EqualsAndHashCode

@davidmorgan
Copy link
Collaborator

You can do that using the compare tag for fields. For convenience, set the default for the whole class to false:

https://pub.dev/documentation/built_value/latest/built_value/BuiltValue/defaultCompare.html

then set it for the id field to true:

https://pub.dev/documentation/built_value/latest/built_value/BuiltValueField/compare.html

@eEQK
Copy link

eEQK commented Apr 21, 2021

I have no idea how I've missed that... Thanks for the reply, you're the best! ❤

@robcapo
Copy link

robcapo commented Dec 6, 2021

I'd like to use DeepCollectionEquality to compare one of the fields on my Built class (and to compute its hashCode). Is there any way to customize this given that we can't override operator== and hashCode?

@dave26199
Copy link

You should usually use built_collection classes with built_value, those do deep comparisons and hash codes already. They also cache hash values to optimize.

@robcapo
Copy link

robcapo commented Dec 6, 2021

So at the top level I have a built value class, which has a BuiltMap<Object, bool> on it. This is all great because it's serializable, but now I want to use DeepCollectionEquality to compare the keys in the map (which are out of my control). In order to do that, I defined:

class DeepCollectionEqualityKey {
  DeepCollectionEqualityKey(this.key);

  Object key;

  @override
  bool operator ==(Object other) =>
      other is DeepCollectionEqualityKey &&
      DeepCollectionEquality().equals(key, other.key);

  @override
  int get hashCode => DeepCollectionEquality().hash(key);
}

and I changed the map to BuiltMap<DeepCollectionEqualityKey, bool>. Now the map is working properly for comparison, but serialization is broken, because there's no serializer for DeepCollectionEqualityKey.

So I tried to make DeepCollectionEqualityKey a built value, but built doesn't allow you to override == and hashCode. Looking at the operator== implementation of BuiltValue, it looks like it does use operator== on all of its members, but since the keys are out of my control, I can't confirm that they will use built_collection where necessary.

My thinking is that I probably have to create a custom serializer instead, but just wanted to check if there's a better way to do it with built_value

@dave26199
Copy link

I'm not sure I follow ... if the keys are out of your control, how is it that they are already serializable?

built_value should serialize to built_collection types, which would make the compares work as desired.

@robcapo
Copy link

robcapo commented Dec 8, 2021

Yeah I think it's a fair question (the short answer is that we don't really have those guarantees aside from documentation and testing infrastructure). I guess we rely heavily enough on Built for serialization, that we should probably just rely on it for equality as well. Thanks for talking through it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants