-3
\$\begingroup\$

Does it bad practice inherit class to reuse toString/equals/hashCode (that actually based on reflection and use actual class field for it)?

For example:

public abstract class AbstractObject {

private List<Field> collectTransientFields() {
    //
    return Arrays.stream(getClass().getDeclaredFields())
            .filter((f) -> f.getAnnotation(Transient.class) != null)
            .collect(Collectors.toList());
}

@Override
public String toString() {
    return ToStringBuilder.reflectionToString(this, DEFAULT_STYLE);
}


@Override
public boolean equals(Object o) {
    //
    List<String> transients = collectTransientFields().stream()
            .map(Field::getName)
            .collect(Collectors.toList());
    return EqualsBuilder.reflectionEquals(this, o, transients);
}

@Override
public int hashCode() {
    List<String> transients = collectTransientFields().stream()
            .map(Field::getName)
            .collect(Collectors.toList());
    return HashCodeBuilder.reflectionHashCode(this, transients);
}

private List<Field> collectTransientFields() {
    //
    return Arrays.stream(getClass().getDeclaredFields())
            .filter((f) -> f.getAnnotation(Transient.class) != null)
            .collect(Collectors.toList());
}

class A extends AbstractObject
class B extends AbstractObject

I benchmarked this implementation of hashCode and IDE-generated one. IDE-generated 10 000 faster for two-field class.

On the other hand there is even library (jakarta) that provide this reflection functionality.

So I have two question:

  1. Is it ok to inherit classes for reuse toString/equals/hashCode functionality
  2. Is it ok to use reflection-based implementation of toString/equals/hashCode instead of code-generated or human-written one
\$\endgroup\$

closed as off-topic by Vogel612, Toby Speight, Stephen Rauch, Sᴀᴍ Onᴇᴌᴀ, t3chb0t Feb 21 '18 at 8:32

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. The example code that you have posted is not reviewable in this form because it leaves us guessing at your intentions. Unlike Stack Overflow, Code Review needs to look at concrete code in a real context. Please see Why is hypothetical example code off-topic for CR? \$\endgroup\$ – Vogel612 Feb 19 '18 at 12:02
  • \$\begingroup\$ Why even is toString a hot path in your code in the first place??? \$\endgroup\$ – Vogel612 Feb 19 '18 at 12:03
  • \$\begingroup\$ As Vogel said, this hypothetical question doesn't show us when and why you would ever want to use this. Could you at least provide us with an example of a class + subclass in which this construction makes sense to you? (in actual concrete code, not a hypothetical example please). That way, we can either confirm, or tell you how to do it better instead. \$\endgroup\$ – Imus Feb 19 '18 at 12:20
  • \$\begingroup\$ @Vogel612 my first question here. I made some mistakes. sorry about that. Is updated version clearer? \$\endgroup\$ – Sergii Getman Feb 19 '18 at 13:05
  • \$\begingroup\$ @Imus my question should be edited. Does updated version look better? \$\endgroup\$ – Sergii Getman Feb 19 '18 at 13:06
1
\$\begingroup\$

I don't see the reason to use this class. I don't really use the equals/hashcode/toString methods that often to start with and if I would use them I prefer to make them explicit in that class. If at some point someone is interested in knowing when 2 instances are considered equal they can check the class itself rather than some weird indirection with reflection.

Does it really bother you that much to have 3 trivially auto-generated methods in a class? Especially if you would still override the toString to write something meaningful in the given context rather than just printing out all fields.


To answer your question

Is it ok to inherit classes for reuse toString/equals/hashCode functionality

sure, but don't forget you can only inherit a single class. So if you want to extend a class that doesn't already extend your special Object class it's impossible.

Then again, in most cases it's better to be explicit about it. If you want your user of a certain class to know that he can safely use the instances in lists, hashmaps, ... or is able to print them out somewhat "nicely" it may be preferred to show it by having the methods inside that class definition itself.

Is it ok to use reflection-based implementation of toString/equals/hashCode instead of code-generated or human-written one

I would never do it myself. I have a personal dislike of all reflection unless if it really really helps. A good example is being able to transform a POJO into JSON or XML by using reflection on the fields. This turns a tedious task of writing transformers and keeping them up to date with all changes of the class into a trivial thing with at best adding some annotations.

For your case it's the only way to implement it and at the same time completely hide these methods from the extending classes. Well almost completely since you still need to add an extends AbstractObject to the signature of the class itself.

But do you really gain much from doing so?

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.