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

Replace StringBuffer with StringBuilder #243

Merged
merged 1 commit into from Jan 8, 2015

Conversation

Projects
None yet
3 participants
@alexismeneses
Contributor

alexismeneses commented Jan 5, 2015

Take advantage of Java 5 using StringBuilder where applicable

  • replacement is done when StringBuffer is used as a local variable
  • public methods using StringBuffer are kept but deprecated in favor of the same using StringBuilder. Cases can be found in org.postgresql.core.Utils. New public methods using StringBuilder do not use the same name as their StringBuffer counterpart otherwise it would prevent existing code using null as a method argument to compile.
Replace usage of StringBuffer with its drop-in replacement StringBuil…
…der where applicable

 - replacement is done when StringBuffer is used as a local variable
 - public APIs using StringBuffer are kept but deprecated in favor of the same using StringBuilder (note a method name change is required not to prevent code using null as parameter to compile)
@sehrope

This comment has been minimized.

Contributor

sehrope commented Jan 6, 2015

First off I think this is awesome. I've been thinking about this every time I poke through the code. Nice to see someone got around to it.

I don't see a point to maintaining two copies of the public methods. They may be public, but they're still internal to the driver. The only true public API of the driver is the core JDBC API and the extensions available via casting to PGConnection (ex: the CopyManager API).

Nobody should be using the "internal" public methods (ex: the Utils class) and, if they are, it'd be their responsibility to update their code to use a StringBuilder instead. I say this as someone who has used those internal helper functions in the past.

This is especially true if it'll be done as a new major release of the driver. Users that are upgrading to a 9.4 (or I guess 9.5?) driver can be expected to compile/test it with their app. It'll immediately be apparent what they need to fix.

@davecramer @ringerc : What do you guys think?

@davecramer

This comment has been minimized.

Member

davecramer commented Jan 6, 2015

Ya, I see no reason to keep the old methods around.

Dave Cramer

On 5 January 2015 at 21:24, Sehrope Sarkuni notifications@github.com
wrote:

First off I think this is awesome. I've been thinking about this every
time I poke through the code. Nice to see someone got around to it.

I don't see a point to maintaining two copies of the public methods. They
may be public, but they're still internal to the driver. The only true
public API of the driver is the core JDBC API and the extensions available
via casting to PGConnection (ex: the CopyManager API).

Nobody should be using the "internal" public methods (ex: the Utils class)
and, if they are, it'd be their responsibility to update their code to use
a StringBuilder instead. I say this as someone who has used those internal
helper functions in the past.

This is especially true if it'll be done as a new major release of the
driver. Users that are upgrading to a 9.4 (or I guess 9.5?) driver can be
expected to compile/test it with their app. It'll immediately be apparent
what they need to fix.

@davecramer https://github.com/davecramer @ringerc
https://github.com/ringerc : What do you guys think?


Reply to this email directly or view it on GitHub
#243 (comment).

@alexismeneses

This comment has been minimized.

Contributor

alexismeneses commented Jan 6, 2015

Just note that the real logic is done in a shared private method. Public methods just delegate the job to it whether the provided arg is a StringBuilder or StringBuffer. So there not really two copies to maintain actually.

davecramer added a commit that referenced this pull request Jan 8, 2015

Merge pull request #243 from alexismeneses/Java5
Replace StringBuffer with StringBuilder for performance

@davecramer davecramer merged commit 5060291 into pgjdbc:master Jan 8, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment