Project

General

Profile

Task #5867

Implement hashCode() and equals(...) for VariableContainer/MetaInfo and it's children

Added by Igor Melnichenko over 5 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Model
Target version:
Start date:
04/21/2015
Due date:
% Done:

100%

Estimated time:
Detected in build:
svn
Published in build:
0.2.1

Description

Implement hashCode() and equals(...) for VariableContainer and MetaInfo

History

#1

Updated by Sergey Smolov over 5 years ago

  • Status changed from New to Open
#2

Updated by Sergey Smolov over 5 years ago

  • Status changed from Open to Resolved
  • % Done changed from 0 to 100

Done in r1872.

#3

Updated by Igor Melnichenko over 5 years ago

  • Status changed from Resolved to Verified
#4

Updated by Sergey Smolov over 5 years ago

  • Status changed from Verified to Open
  • % Done changed from 100 to 0

One cannot simply implement such methods like equals/hashcode in classes that are extended in other, since it causes a lot of errors.

I've commented these methods for a while, I need to think about them.

r1876

#5

Updated by Sergey Smolov over 5 years ago

  • Subject changed from Implement hashCode() and equals(...) for VariableContainer and MetaInfo to Implement hashCode() and equals(...) for VariableContainer/MetaInfo and it's children
#6

Updated by Sergey Smolov over 5 years ago

  • Category set to 125
#7

Updated by Sergey Smolov over 5 years ago

  • Status changed from Open to Feedback

In fact I don't catch the main idea.

Is it OK that you won't be able to call hashcode/equals methods of VariableContainer/MetaInfo classes since they're abstract?

Every time you will deal with other classes that extend VariableContainer/MetaInfo and they will have their own implementations of hashcode/equals methods.

#8

Updated by Igor Melnichenko over 5 years ago

It's much more simple and clear to provide hashCode() and equals() implementations in VariableContainer/MetaInfo than write boilerplate hashCode() and equals() code for fields inherited from VariableContainer/MetaInfo in all VariableContainer/MetaInfo subclasses.

#9

Updated by Sergey Smolov over 5 years ago

I've proposed the following scheme.

VariableDeclarationNode (today it is the VariableContainer, this name is not so good) - contains VariableContainer (this class incapsulates a collection for variable declarations)
MetaInfoNode (today it is MetaInfo) - contains MetaInfoContainer (this class incapsulates Map<MetaInfoType, MetaInfoValue>)

If I implement equals/hashcode methods for MetaInfoContainer\VariableContainer I won't be able to re-implement them in classes that extend VariableDeclarationNode/MetaInfoNode.

Of course the code of such methods is boilerplate but there is non need to override hashcode/equals for extending classes.

#10

Updated by Igor Melnichenko over 5 years ago

Sergey Smolov wrote:

If I implement equals/hashcode methods for MetaInfoContainer\VariableContainer I won't be able to re-implement them in classes that extend VariableDeclarationNode/MetaInfoNode.

I don't understand why do you want to reimplement them. MetaInfoContainer\VariableContainer are the same for all subclasses of VariableDeclarationNode/MetaInfoNode so the logic of their equals()/hashCode() is expected to be the same.

#11

Updated by Sergey Smolov over 5 years ago

Today I use default implementations of equals\hashcode methods for objects of classes that extend VariableContainer/MetaInfo.

But when I implement methods you want I'll need to implement equal/hashcode also for all the child classes. Why? Suppose I need to check rather two BasicBlock nodes are equal. In your situation they will be equal only when their MetaInfo 'equals' method return 'true'. Don't you think it is really strange?

#12

Updated by Sergey Smolov over 5 years ago

To feel the problem you may generate equals/hashcode methods for MetaInfo class (using Eclipse IDE, for example) and try to run CgaaEfsmTransformerTestCase, for example. It will fail because of the problem I've told above.

#13

Updated by Igor Melnichenko over 5 years ago

Sergey Smolov wrote:

Today I use default implementations of equals\hashcode methods for objects of classes that extend VariableContainer/MetaInfo.

But when I implement methods you want I'll need to implement equal/hashcode also for all the child classes. Why? Suppose I need to check rather two BasicBlock nodes are equal. In your situation they will be equal only when their MetaInfo 'equals' method return 'true'. Don't you think it is really strange?

I think it is the only right behaviour. If two basic blocks are equal, they should contain the same meta information. But maybe I have a wrong opinion about semantics of basic blocks.
Anyway, I'm strongly for equals()/hashCode() implementation. If you want, I'll implement these methods carefully without affecting behaviour of existing subclasses.

#14

Updated by Sergey Smolov over 5 years ago

Igor Melnichenko wrote:

Sergey Smolov wrote:

Today I use default implementations of equals\hashcode methods for objects of classes that extend VariableContainer/MetaInfo.

But when I implement methods you want I'll need to implement equal/hashcode also for all the child classes. Why? Suppose I need to check rather two BasicBlock nodes are equal. In your situation they will be equal only when their MetaInfo 'equals' method return 'true'. Don't you think it is really strange?

I think it is the only right behaviour. If two basic blocks are equal, they should contain the same meta information. But maybe I have a wrong opinion about semantics of basic blocks.

It is wrong because in this case you say that basic blocks with two and three assignments can be equal (if they have equal meta-information).

Anyway, I'm strongly for equals()/hashCode() implementation. If you want, I'll implement these methods carefully without affecting existing subclasses.

i wonder how you are going to do this? Every time when child class will be compared with another child class, you will call either 'equals' method of this class or the 'equals' method of the superclass.

#15

Updated by Igor Melnichenko over 5 years ago

Sergey Smolov wrote:

Igor Melnichenko wrote:

Sergey Smolov wrote:

Today I use default implementations of equals\hashcode methods for objects of classes that extend VariableContainer/MetaInfo.

But when I implement methods you want I'll need to implement equal/hashcode also for all the child classes. Why? Suppose I need to check rather two BasicBlock nodes are equal. In your situation they will be equal only when their MetaInfo 'equals' method return 'true'. Don't you think it is really strange?

I think it is the only right behaviour. If two basic blocks are equal, they should contain the same meta information. But maybe I have a wrong opinion about semantics of basic blocks.

It is wrong because in this case you say that basic blocks with two and three assignments can be equal (if they have equal meta-information).

It is not my point. My point is that equality of meta information is necessary (but insufficient) for equality of basic blocks.

#16

Updated by Sergey Smolov over 5 years ago

  • Status changed from Feedback to Open

Ok

#17

Updated by Igor Melnichenko over 5 years ago

To preserve the current behaviour it is enough to add in all subclasses without equals/hashCode implementation the nearest implementation from superclass.

#18

Updated by Sergey Smolov over 5 years ago

Igor Melnichenko wrote:

To preserve the current behavior it is enough to add in all subclasses without equals/hashCode implementation the nearest implementation from super-class.

To preserve the current behavior you need to use default implementation of equals/hashcode methods (from Object class) in sub-classes an do not call the implementation from super-class at all.

Because the current behavior is based on default implementations.

#19

Updated by Igor Melnichenko over 5 years ago

Igor Melnichenko wrote:

To preserve the current behaviour it is enough to add in all subclasses without equals/hashCode implementation the nearest implementation from superclass.

It is exactly what I said as Object is the nearest superclass with hashCode/equals implementation.

#20

Updated by Sergey Smolov over 5 years ago

Igor Melnichenko wrote:

Igor Melnichenko wrote:

To preserve the current behaviour it is enough to add in all subclasses without equals/hashCode implementation the nearest implementation from superclass.

It is exactly what I said as Object is the nearest superclass with hashCode/equals implementation.

But in which way you are going to use methods from abstract classes if I incapsulate them by overridden ones?

#21

Updated by Igor Melnichenko over 5 years ago

Sergey Smolov wrote:

Igor Melnichenko wrote:

Igor Melnichenko wrote:

To preserve the current behaviour it is enough to add in all subclasses without equals/hashCode implementation the nearest implementation from superclass.

It is exactly what I said as Object is the nearest superclass with hashCode/equals implementation.

But in which way you are going to use methods from abstract classes if I incapsulate them by overridden ones?

I will use them only in Efsm and EfsmContainer which will have direct access to these methods (if I understood the question correctly).

#22

Updated by Sergey Smolov over 5 years ago

Aaah, I see.

So, we have here the following situation: an abstract class A, my sub-class B, you sub-class C. You want to use methods of A in C. But the problem lies in the fact that these methods are special and I will use them with necessarity in class B (although I don't want to feel an influence of A in B classes)!

May be I just make some fields of A as protected and you will construct you own hashcode/equals methods in the C classes you want to compare? In such case there is non need in equals/hashcode methods in either A or B classes (and in B classes I will continue to use default Object implementations), you will implement them only in C.

#23

Updated by Sergey Smolov over 5 years ago

  • Status changed from Open to Feedback
#24

Updated by Igor Melnichenko over 5 years ago

Sergey Smolov wrote:

Aaah, I see.

So, we have here the following situation: an abstract class A, my sub-class B, you sub-class C. You want to use methods of A in C. But the problem lies in the fact that these methods are special and I will use them with necessarity in class B (although I don't want to feel an influence of A in B classes)!

May be I just make some fields of A as protected and you will construct you own hashcode/equals methods in the C classes you want to compare? In such case there is non need in equals/hashcode methods in either A or B classes (and in B classes I will continue to use default Object implementations), you will implement them only in C.

This approach has two disadvantages:
1) it is hard to maintain this architecture as future changes in A should be reflected in each C.
2) the same boilerplate code in each C.

Now there are only two C but I think there will be another subclasses of A which will require equals/hashCode overriding so impact of these disadvantages will grow over time.

#25

Updated by Sergey Smolov over 5 years ago

  • Status changed from Feedback to Open

Ok

#26

Updated by Sergey Smolov over 5 years ago

  • Status changed from Open to Feedback

I have one more proposal for you. Can you create your own abstract sub-classes for MetaInfo/VariableContainer abstract classes and implement equals/hashcode methods inside them?

I've tried to implement your scheme and I have the problem that in huge number of Retrascope components there is an implicit contract that CFG nodes can be equal (and have equal hashcodes) only when their references are equal. So after the implementation of equals/hashcode methods in our common abstract classes I must override these methods in every my sub-classes. It is really simple for equals method, but for hashcode I have no idea how to call the Object class implementation of hashcode.

#27

Updated by Igor Melnichenko over 5 years ago

Sergey Smolov wrote:

I have one more proposal for you. Can you create your own abstract sub-classes for MetaInfo/VariableContainer abstract classes and implement equals/hashcode methods inside them?

I've tried to implement your scheme and I have the problem that in huge number of Retrascope components there is an implicit contract that CFG nodes can be equal (and have equal hashcodes) only when their references are equal. So after the implementation of equals/hashcode methods in our common abstract classes I must override these methods in every my sub-classes. It is really simple for equals method, but for hashcode I have no idea how to call the Object class implementation of hashcode.

This is the default hashCode() implementation: http://docs.oracle.com/javase/7/docs/api/java/lang/System.html#identityHashCode(java.lang.Object)
Are your proposal still valid?

#28

Updated by Sergey Smolov over 5 years ago

Thank you for the default hashCode() implemenation javadoc. That solves the technical part of the problem. Now I'm waiting for the jUnit tests to be finished.

The conceptual part of the problem lies in the fact that I've added the identical implementations of equals/hashCode methods to 13 (!) classes.

As for the my proposal will need only to create two abstract sub-classes and nothing else (may be just improve Efsm/EfsmContainer classes).

So I see 13+2=15 classes in your approach against 4 in mine.

Are you still think that your approach is much more elegant?

#29

Updated by Igor Melnichenko over 5 years ago

Sergey Smolov wrote:

Thank you for the default hashCode() implemenation javadoc. That solves the technical part of the problem. Now I'm waiting for the jUnit tests to be finished.

The conceptual part of the problem lies in the fact that I've added the identical implementations of equals/hashCode methods to 13 (!) classes.

As for the my proposal will need only to create two abstract sub-classes and nothing else (may be just improve Efsm/EfsmContainer classes).

So I see 13+2=15 classes in your approach against 4 in mine.

Are you still think that your approach is much more elegant?

Yes. I think that number of subclasses which must be compared by references will only reduce over time while "logical" comparison will be more common.

#30

Updated by Sergey Smolov over 5 years ago

  • Status changed from Feedback to Open
#31

Updated by Sergey Smolov over 5 years ago

  • Status changed from Open to Resolved
  • % Done changed from 0 to 100

Done in r1920.

#32

Updated by Igor Melnichenko about 5 years ago

  • Status changed from Resolved to Verified
#33

Updated by Sergey Smolov over 4 years ago

  • Status changed from Verified to Closed
  • Published in build set to 0.2.1

Also available in: Atom PDF