Skip to content

Add a lazy assert to prevent unnecessary function calls. #1876

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.

Sign up for GitHub

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

Closed
klee97 wants to merge 1 commit into from
Closed

Add a lazy assert to prevent unnecessary function calls. #1876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/main/java/graphql/Assert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package graphql;

import java.util.Arrays;
import java.util.Collection;
import java.util.function.Supplier;

import static java.lang.String.format;

Expand Down Expand Up @@ -76,6 +78,13 @@ public static void assertTrue(boolean condition, String format, Object... args)
throw new AssertException(format(format, args));
}

public static void assertTrueLazily(boolean condition, String format, Supplier<?>... args) {
if (condition) {
return;
}
throw new AssertException(format(format, Arrays.stream(args).map(Supplier::get).toArray()));
}

@bbakerman bbakerman May 19, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if its just a string that is laziyl supplied and the caller does the formatting

    assertTrue(x == y, () -> String.format("This is the message %s, sArg1));

this way we can re-use the simple assertTrue name and migrate to it in more places

@andimarek andimarek May 19, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes lets do it this way @bbakerman

public static void assertTrue(boolean condition) {
if (condition) {
return;
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/graphql/schema/GraphQLNonNull.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@

import java.util.Collections;
import java.util.List;
import java.util.function.Supplier;

import static graphql.Assert.assertNotNull;
import static graphql.Assert.assertTrue;
import static graphql.Assert.assertTrueLazily;

/**
* A modified type that indicates there the underlying wrapped type will not be null.
Expand Down Expand Up @@ -46,8 +47,8 @@ public GraphQLNonNull(GraphQLType wrappedType) {
}

private void assertNonNullWrapping(GraphQLType wrappedType) {
assertTrue(!GraphQLTypeUtil.isNonNull(wrappedType), String.format("A non null type cannot wrap an existing non null type '%s'",
GraphQLTypeUtil.simplePrint(wrappedType)));
assertTrueLazily(!GraphQLTypeUtil.isNonNull(wrappedType), "A non null type cannot wrap an existing non null type '%s'",
() -> GraphQLTypeUtil.simplePrint(wrappedType));
}

@Override
Expand Down
25 changes: 25 additions & 0 deletions src/test/groovy/graphql/AssertTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package graphql

import spock.lang.Specification

import java.util.function.Supplier

class AssertTest extends Specification {
def "assertNull should not throw on none null value"() {
when:
Expand Down Expand Up @@ -120,6 +122,29 @@ class AssertTest extends Specification {
"code" | null || "code"
}

def "assertTrueLazily should not throw on true value"() {
when:
Assert.assertTrueLazily(true, "error message")

then:
noExceptionThrown()
}

def "assertTrueLazily with error message should throw on false value with formatted message"() {
when:
Assert.assertTrueLazily(false, format, { _ -> arg} as Supplier)

then:
def error = thrown(AssertException)
error.message == expectedMessage

where:
format | arg || expectedMessage
"error %s" | "msg" || "error msg"
"code %d" | 1 || "code 1"
"code" | null || "code"
}

def "assertValidName should not throw on valid names"() {
when:
Assert.assertValidName(name)
Expand Down