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

javers.compare throw class cast exception #1030

Closed
mhelvo opened this issue Oct 22, 2020 · 5 comments
Closed

javers.compare throw class cast exception #1030

mhelvo opened this issue Oct 22, 2020 · 5 comments

Comments

@mhelvo
Copy link

mhelvo commented Oct 22, 2020

Clear description of my expectations versus reality
No exception should be thrown

Steps To Reproduce
I have a **runnable test case ** which isolates the bug and allows Javers Core Team to easily reproduce it. I have pushed this test case to my fork of this repository:

https://github.com/mhelvo/javers/blob/master/javers-core/src/test/java/org/javers/core/cases/ListWithChildClassesTest.java

Javers' Version
5.**

Additional context
...

@bartoszwalacik
Copy link
Member

Interesting bug, thanks for the good report

@bartoszwalacik
Copy link
Member

bartoszwalacik commented Oct 22, 2020

This case can't be solved in Javers, you can't compare String to List.
all that can be done is better error message or skipping this properties.
Btw I think that having two properties with totally different types but with the same name in the same class hierarchy is not very elegant.

@bartoszwalacik
Copy link
Member

fixed in 5.13.2

@mhelvo
Copy link
Author

mhelvo commented Oct 26, 2020

I think it will be very useful to check types of the items in the list. In this case I do not compare a List with a String. I compare two lists containing fields. I assume I should be quite easy to first check it the item contain the same type. If not there is a difference between those items.

But even if you want to check between the same property names, a simple check on property type would fix this issue.

A the moment it is a very tricky one. Just throwing an exception will only occur at runtime. And if a developer is not aware of this, the issue might be first recognized on production. A check the other way around does not trigger this exception.

About the solution is elegant or not, is a matter of taste and not very relevant in this case. In my opinion each child class should uses the property names they that fit, and should not depend on the fact if the name is already used in another class or not.

@bartoszwalacik
Copy link
Member

This issue is solved by simple type checking

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

2 participants