Skip to content

Conversation

@linev
Copy link
Member

@linev linev commented May 17, 2017

Included are several classes; testing most kinds of streamer elements.
Will work only after merging root-project/root#580

@linev linev requested a review from pcanal May 17, 2017 09:15
$(TestDiff)

RootClasses: RootClasses.log
$(TestDiff)
Copy link
Member

Choose a reason for hiding this comment

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

Please add

RootClasses.log: test_classes_h.$(DllSuf)

for all the cases that uses test_classes.h+ to allow for parallel builds.

Copy link
Member Author

Choose a reason for hiding this comment

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

I add correspondent rule in Makefile

}
virtual ~TJsonEx3()
{
//delete[] fBool;
Copy link
Member

Choose a reason for hiding this comment

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

Why is the array not deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was due to missing copy constructor.
I used this class in std::vector and arrays pointers were copied
Now class should work correctly.

TJsonEx6(bool setvalues = false)
{
for (int n=0;n<3;n++) {
fPtr1[n] = 0;
Copy link
Member

Choose a reason for hiding this comment

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

whereever you have 0 meaning null pointer, can you use 'nullptr'? thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@pcanal pcanal merged commit a0c9175 into root-project:master May 27, 2017
@linev
Copy link
Member Author

linev commented May 27, 2017

Philippe - you are too fast,
Test will fail with master branch.
You first should merge root-project/root#580

@linev linev deleted the json branch May 27, 2017 19:15
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