You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by oreissig <gi...@git.apache.org> on 2016/03/12 23:08:21 UTC
[GitHub] groovy pull request: make private fields final where possible
GitHub user oreissig opened a pull request:
https://github.com/apache/groovy/pull/292
make private fields final where possible
As discussed in PR #282 I repropose the changes from there in individual pull requests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/oreissig/groovy final-fields
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/groovy/pull/292.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #292
----
commit ce3238210a98ec4dc418c20b007b0eaf303e3f88
Author: oreissig <or...@gmail.com>
Date: 2016-03-12T21:28:30Z
make private fields final where possible
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request #292: make private fields final where possible
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/groovy/pull/292
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: make private fields final where possible
Posted by jwagenleitner <gi...@git.apache.org>.
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/292#discussion_r56097572
--- Diff: subprojects/groovy-sql/src/main/java/groovy/sql/BatchingStatementWrapper.java ---
@@ -34,10 +34,10 @@
* automatically. If batchSize is zero, then no batching is performed.
*/
public class BatchingStatementWrapper extends GroovyObjectSupport {
- private Statement delegate;
+ private final Statement delegate;
protected int batchSize;
protected int batchCount;
- protected Logger log;
+ protected final Logger log;
--- End diff --
While it may not be likely, there could be some subclasses that set this so I think it might be good to avoid changing `protected` fields.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: make private fields final where possible
Posted by jwagenleitner <gi...@git.apache.org>.
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/292#discussion_r56733877
--- Diff: subprojects/groovy-macro/src/main/groovy/org/codehaus/groovy/macro/matcher/internal/MatchingConstraintsBuilder.groovy ---
@@ -24,7 +24,7 @@ import org.codehaus.groovy.macro.matcher.TreeContext
import org.codehaus.groovy.syntax.Token
class MatchingConstraintsBuilder {
- Set<String> placeholders = new LinkedHashSet<>()
+ final Set<String> placeholders = new LinkedHashSet<>()
--- End diff --
Since this is a Groovy object this is changing a public property to final and I don't think should be done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: make private fields final where possible
Posted by jwagenleitner <gi...@git.apache.org>.
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/292#discussion_r56734403
--- Diff: src/main/org/codehaus/groovy/ast/expr/GStringExpression.java ---
@@ -32,13 +32,15 @@
*/
public class GStringExpression extends Expression {
- private String verbatimText;
- private List<ConstantExpression> strings = new ArrayList<ConstantExpression>();
- private List<Expression> values = new ArrayList<Expression>();
+ private final String verbatimText;
+ private final List<ConstantExpression> strings;
+ private final List<Expression> values;
public GStringExpression(String verbatimText) {
this.verbatimText = verbatimText;
super.setType(ClassHelper.GSTRING_TYPE);
+ this.strings = new ArrayList<ConstantExpression>();
+ this.values = new ArrayList<Expression>();
--- End diff --
This would cause a breaking change if any code relied on `#getStrings` or `#getValues` returning `null`. I would avoid this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: make private fields final where possible
Posted by jwagenleitner <gi...@git.apache.org>.
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/292#discussion_r56734729
--- Diff: src/main/org/codehaus/groovy/runtime/ProcessGroovyMethods.java ---
@@ -433,7 +433,7 @@ public static Process or(final Process left, final Process right) throws IOExcep
* @since 1.0
*/
protected static class ProcessRunner implements Runnable {
- Process process;
+ final Process process;
--- End diff --
This could be a breaking change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: make private fields final where possible
Posted by shils <gi...@git.apache.org>.
Github user shils commented on a diff in the pull request:
https://github.com/apache/groovy/pull/292#discussion_r56113728
--- Diff: src/main/groovy/grape/GrapeIvy.groovy ---
@@ -76,11 +76,11 @@ class GrapeIvy implements GrapeEngine {
Set<String> resolvedDependencies
Set<String> downloadedArtifacts
// weak hash map so we don't leak loaders directly
- Map<ClassLoader, Set<IvyGrabRecord>> loadedDeps = new WeakHashMap<ClassLoader, Set<IvyGrabRecord>>()
+ final Map<ClassLoader, Set<IvyGrabRecord>> loadedDeps = new WeakHashMap<ClassLoader, Set<IvyGrabRecord>>()
--- End diff --
making these fields final will break binary compatibility since their synthetic public setters won't be generated
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] groovy pull request: make private fields final where possible
Posted by jwagenleitner <gi...@git.apache.org>.
Github user jwagenleitner commented on a diff in the pull request:
https://github.com/apache/groovy/pull/292#discussion_r56096947
--- Diff: src/main/org/codehaus/groovy/control/MultipleCompilationErrorsException.java ---
@@ -27,7 +27,7 @@
public class MultipleCompilationErrorsException extends
CompilationFailedException {
- protected ErrorCollector collector;
+ protected final ErrorCollector collector;
--- End diff --
There could be subclasses that set this field.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---