Retrascope

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.

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.

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.

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?

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.

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.

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.

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.

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

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.

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.

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?

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).

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.

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.

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?

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?

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.