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

Optimize memory consumption in junit.framework.Assert #820

Merged
merged 2 commits into from Feb 15, 2014
Merged

Optimize memory consumption in junit.framework.Assert #820

merged 2 commits into from Feb 15, 2014

Conversation

vbauer
Copy link
Contributor

@vbauer vbauer commented Feb 13, 2014

New class org.junit.Assert already contains optimized assert methods (with Long.valueOf etc), for example:

    static public void assertEquals(String message, long expected, long actual) {
        if (expected != actual) {
            failNotEquals(message, Long.valueOf(expected), Long.valueOf(actual));
        }
    }

Old (deprecated) version of this class (junit.framework.Assert) contains not-optimized methods, for example:

    static public void assertEquals(String message, long expected, long actual) {
        assertEquals(message, new Long(expected), new Long(actual));
    }

Whole class is deprecated, but I sure that this methods are still used in many projects, so I suggest to optimize them, until they are presented in JUnit.

@@ -117,7 +117,7 @@ static public void assertEquals(String message, double expected, double actual,
return;
}
if (!(Math.abs(expected - actual) <= delta)) {
failNotEquals(message, new Double(expected), new Double(actual));
failNotEquals(message, Double.valueOf(expected), Double.valueOf(actual));
Copy link
Member

Choose a reason for hiding this comment

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

This actually isn't an optimization; Double.valueOf(double d) just calls 'new Double(d)'

Let's just remove this change. Since this class is deprecated, I want to minimize changes to it wherever possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely agree, I've just repeat the same approach, that was used in new Assert class to work with Float and Double types. It is not a problem to revert it. :)

@kcooney
Copy link
Member

kcooney commented Feb 13, 2014

Thanks for the contribution. I was almost tempted to close this without pulling it; part of the benefit of deprecating a class is that we can reduce the maintenance time by avoiding unnecessary changes to the class. But this looks really safe (famous last words :-)

@vbauer
Copy link
Contributor Author

vbauer commented Feb 13, 2014

Yes, it should be safe. Will it be accepted if I revert changes with Float and Double types?

@kcooney
Copy link
Member

kcooney commented Feb 14, 2014

I'd be happy to pull if you revert the changes with Float and Double

@vbauer
Copy link
Contributor Author

vbauer commented Feb 14, 2014

Ok, some changes were reverted

kcooney added a commit that referenced this pull request Feb 15, 2014
Optimize memory consumption in junit.framework.Assert
@kcooney kcooney merged commit eb8b5ee into junit-team:master Feb 15, 2014
@kcooney
Copy link
Member

kcooney commented Feb 15, 2014

Thanks!

@vbauer
Copy link
Contributor Author

vbauer commented Feb 15, 2014

Thank you for merge!

@stefanbirkner stefanbirkner added this to the 4.12 milestone Feb 15, 2014
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.

None yet

3 participants