Skip to content

move generation of additional types to an iterative approach #619

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

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

tapaderster
Copy link
Member

📝 Description

Changed the current recursive approach of adding the additional types to iterative one.

This approach saves:

  • a bunch of toSet() calls which internally means creation of new LinkedHashSet
  • .plus() calls which also creates a new LinkedHashSet and does an addAll() internally.

@tapaderster tapaderster added type: refactor Code changes that have no impact on users changes: patch Changes require a patch version labels Feb 26, 2020
val graphqlTypes = currentlyProcessedTypes.map { generateGraphQLType(this, it) }.toSet()
val currentlyProcessedTypes = mutableSetOf<KType>()
val graphqlTypes = mutableSetOf<GraphQLType>()
do {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think you want to use while loop instead of do as additional types could be empty

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops, merged to fast. Yes it would execute the first time, it would iterate over an empty set so not doing to much, but a while could be clearer

@smyrick smyrick merged commit fccb5f1 into ExpediaGroup:master Feb 26, 2020
currentlyProcessedTypes.addAll(this.additionalTypes)
this.additionalTypes.clear()
graphqlTypes.addAll(currentlyProcessedTypes.map { generateGraphQLType(this, it) })
currentlyProcessedTypes.clear()
Copy link
Collaborator

@dariuszkuc dariuszkuc Feb 26, 2020

Choose a reason for hiding this comment

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

is constantly clearing the same object better than recreating the hash set?, e.g.

        val graphqlTypes = mutableSetOf<GraphQLType>()
        while (this.additionalTypes.isNotEmpty()) {
            val currentlyProcessedTypes = HashSet(this.additionalTypes)
            this.additionalTypes.clear()
            graphqlTypes.addAll(currentlyProcessedTypes.map { generateGraphQLType(this, it) })
        }
        return graphqlTypes.toSet()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes: patch Changes require a patch version type: refactor Code changes that have no impact on users
Development

Successfully merging this pull request may close these issues.

3 participants