Skip to content

CAMEL-15390: CollectionStringBuilder - Use JDK util methods instead#4085

Merged
davsclaus merged 2 commits intoapache:masterfrom
zbendhiba:CAMEL-15390
Aug 12, 2020
Merged

CAMEL-15390: CollectionStringBuilder - Use JDK util methods instead#4085
davsclaus merged 2 commits intoapache:masterfrom
zbendhiba:CAMEL-15390

Conversation

@zbendhiba
Copy link
Contributor

No description provided.

}
}
produces = csb.isEmpty() ? null : csb.toString();
produces = producesBuilder.toString().isEmpty() ? null : producesBuilder.toString();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can avoid the 2 x toString as that is a costly operation. We can use length method to know if its empty or not. For example if length > 1 (as delimiter is 1) I think. There is a few other places where toString would be called twice like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The length is 0 by the way, the delimiter is not added. If the value is null, the length gets prefix + suffix, which in this case is empty.

Copy link
Contributor

@bedlaj bedlaj left a comment

Choose a reason for hiding this comment

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

As suggested by Claus. This can be optimized a bit by removing duplicate calls to StringJoiner#toString whenever possible. StringJoiner#toString uses StringBuilder#toString internally, which clones whole String. This String is also not interned, it means it is not reused from String pool and every redundant call to toString wastes memory which needs to be garbage-collected, so it is another reason to avoid duplicate toString calls.

@zbendhiba
Copy link
Contributor Author

Ok

@davsclaus davsclaus merged commit 9a7af71 into apache:master Aug 12, 2020
@zbendhiba zbendhiba deleted the CAMEL-15390 branch August 13, 2020 09:26
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.

4 participants