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

Issue #199 added Object array to LiveGraphFactory to stop throwing ex… #200

Merged

Conversation

dmmiller612
Copy link
Contributor

There are some design choices I made that could easily be changed. One for example, if you look at convertToObjectArray, I added the private function to handle primitive and Object arrays, which you may only want Object arrays.

…ing exceptions when comparing two arrays
@bartoszwalacik
Copy link
Member

thnx, will take a look at it tomorrow

@bartoszwalacik
Copy link
Member

could you give mie write access to https://github.com/dmmiller612/javers? I'd like to put some small changes in this PR

@dmmiller612
Copy link
Contributor Author

You should have write access, now.

@@ -44,6 +45,10 @@ private Object wrapTopLevelContainer(Object handle){
if (handle instanceof Set){
return new SetWrapper((Set)handle);
}
if (handle.getClass().isArray()){
//return new ArrayWrapper(convertToObjectArray(handle));
return new ArrayWrapper((Object[])handle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmmiller612, I've removed this conversion and the tests are still passing, not sure what is it for ...
If this version is ok for you, I can merge

@dmmiller612
Copy link
Contributor Author

The rason I added the conversion is because you can't directly cast Object[] to a primitive[] at least in Java. I don't know much about groovy, and if it boxes the integer to an object. If you are fine just using objects, which is a valid design choice, I am okay with it.

@bartoszwalacik
Copy link
Member

you are right, seems copying is needed as there is no obvious way to do casting. So I've restored it.
I've added more test cases to cover both primitive and object arrays.

bartoszwalacik added a commit that referenced this pull request Sep 20, 2015
…ctory

Issue #199 added Object array to LiveGraphFactory to stop throwing ex…
@bartoszwalacik bartoszwalacik merged commit 03fb3a5 into javers:master Sep 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants