I've made some new warning settings in eclipse. With these new settings I'm facing a strange warning. After reading I got to know what it is but couldn't find a way to remove it.
Here is my problem with the sample code
public class Test {
private String testString;
public void performAction() {
new Thread( new Runnable() {
@Override
public void run() {
testString = "initialize"; // **
}
});
}
}
The line with ** gives me a warning in eclipse
Read access to enclosing field Test.testString is emulated by a synthetic accessor method.
Increasing its visibility will improve your performance.
Problem is, I don't want to change the access modifier of testString
.
Also, don't want to create a getter for it.
What change should be done?
More descriptive example
public class Synthetic
{
private JButton testButton;
public Synthetic()
{
testButton = new JButton("Run");
testButton.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent ae)
{
/* Sample code */
if( testButton.getText().equals("Pause") ) {
resetButton(); // **
} else if( testButton.getText().equals("Run") ) {
testButton.setText("Pause"); // **
}
}
}
);
}
public void reset() {
//Some operations
resetButton();
}
private void resetButton() {
testButton.setText("Run");
}
}
Lines with **
gives me the same warning.
CodePudding user response:
What is a "synthetic" method?
Starting from the Method
class (and it's parent, Member
) we learn that synthetic members are "introduced by the compiler", and that JLS §13.1 will tell us more. It notes:
A construct emitted by a Java compiler must be marked as synthetic if it does not correspond to a construct declared explicitly or implicitly in source code
Since this section is discussing binary compatibility the JVMS is also worth referencing, and JVMS §4.7.8 adds a little more context:
A class member that does not appear in the source code must be marked using a
Synthetic
attribute, or else it must have itsACC_SYNTHETIC
flag set. The only exceptions to this requirement are compiler-generated methods which are not considered implementation artifacts....The
Synthetic
attribute was introduced in JDK 1.1 to support nested classes and interfaces.
In other words, "synthetic" methods are an implementation artifact that the Java compiler introduces in order to support language features that the JVM itself does not support.
What's the problem?
You're running into one such case; you're attempting to access a private
field of a class from an anonymous inner class. The Java language permits this but the JVM doesn't support it, and so the Java compiler generates a synthetic method that exposes the private
field to the inner class. This is safe because the compiler doesn't allow any other classes to call this method, however it does introduce two (small) issues:
- Additional methods are being declared. This shouldn't be a problem for the vast majority of use cases, but if you're working in a constrained environment like Android and are generating a lot of these synthetic methods you may run into issues.
- Access to this field is done indirectly through the synthetic method, rather than directly. This too shouldn't be a problem except for highly performance-sensitive use cases. If you wouldn't want to use a getter method here for performance reasons, you wouldn't want a synthetic getter method either. This is rarely an issue in practice.
In short, they're really not bad. Unless you have a concrete reason to avoid synthetic methods (i.e. you've determined conclusively they are a bottleneck in your application) you should just let the compiler generate them as it sees fit. Consider turning off the Eclipse warning if it's going to bother you.
What should I do about them?
If you really want to prevent the compiler from generating synthetic methods you have a couple of options:
Option 1: Change the permissions
Package-private or protected
fields are accessible to inner classes directly. Especially for something like a Swing application this ought to be fine. But you say you want to avoid this, so on we go.
Option 2: Create a getter
Leave your fields alone, but explicitly create a protected
or public
getter, and use that instead. This is essentially what the compiler ends up doing for you automatically, but now you have direct control over the method's behavior.
Option 3: Use a local variable and share the reference with both classes
This is more code but it's my personal favorite since you're making the relationship between the inner and outer class explicit.
public Synthetic() {
// Create final local instance - will be reachable by the inner class
final JButton testButton = new JButton("Run");
testButton.addActionListener(
new ActionListener() {
public void actionPerformed(ActionEvent ae) {
/* Sample code */
if( testButton.getText().equals("Pause") ) {
resetButton();
} else if( testButton.getText().equals("Run") ) {
testButton.setText("Pause");
}
}
});
// associate same instance with outer class - this.testButton can be final too
this.testButton = testButton;
}
This isn't always actually what you want to do. For instance if testButton
can change to point to a different object later, you'll need to rebuild your ActionListener
again (though that's also nicely more explicit, so arguably this is a feature), but I consider it the option that most clearly demonstrates its intent.
Aside on thread-safety
Your example Test
class is not thread-safe - testString
is being set in a separate Thread
but you're not synchronizing on that assignment. Marking testString
as volatile
would be sufficient to ensure all threads see the update. The Synthetic
example doesn't have this problem since testButton
is only set in the constructor, but since that's the case it would be advisable to mark testButton
as final
.
CodePudding user response:
In your second example it is not necessary to access testButton
directly; you can access it by retrieving the source of the action event.
For the resetButton()
method you can add an argument to pass the object to act upon, if you've done that it is not such a big problem lowering its access restrictions.
CodePudding user response:
Given the context (you assign to the variable once as part of a fairly expensive operation), I don't think you need to do anything.
CodePudding user response:
I think the problem is that you are setting a String on the parent class. This will suffer in performance because the Thread needs to look up to see where that variable is again. I think a cleaner approach is using Callable which returns a String and then do .get() or something that returns the result. After getting the result you can set the data back on to the parent class.
The idea is you want to make sure the Thread only does one thing and only one thing instead of setting variables on other classes. This is a cleaner approach and probably faster because the inner Thread doesn't access anything outside of itself. Which mean less locking. :)
CodePudding user response:
This is one of the rare cases where Java's default visibility (also called "package private") is of use.
public class Test {
/* no private */ String testString;
public void performAction() {
new Thread( new Runnable() {
@Override
public void run() {
testString = "initialize"; // **
}
});
}
}
This will do the following:
testString
is now available to all classes in the same package as the outer class (Test
).- As inner classes are actually generated as
OuterClassPackage.OuterClassName$InnerClassName
, they also reside in the same package. They can therefore access this field directly. - In contrast to making this field
protected
, the default visibility will not make this field available to subclasses (except when they are in the same package of course). You therefore don't pollute your API for external users.
When using private
, javac will instead generate a synthetic accessor, which itself is just a getter method with Java's default visibility as well. So it basically does the same thing, except with a minimal overhead of an additional method.