我做了一些测试，发现我正在使用的 java 编译器（Mac OS X 10.4 上的 javac 1.5.0_19）没有应用我预期的优化。
我用下面的类来测试：
public abstract class Test{
public int singleReturn(){
int ret = 0;
if (cond1())
ret = 1;
else if (cond2())
ret = 2;
else if (cond3())
ret = 3;
return ret;
}
public int multReturn(){
if (cond1()) return 1;
else if (cond2()) return 2;
else if (cond3()) return 3;
else return 0;
}
protected abstract boolean cond1();
protected abstract boolean cond2();
protected abstract boolean cond3();
}
For readable properties there will be a getter method to read the property value. For writable properties there will be a setter method to allow the property value to be updated. [...] Constructs a PropertyDescriptor for a property that follows the standard Java convention by having getFoo and setFoo accessor methods. Thus if the argument name is "fred", it will assume that the reader method is "getFred" and the writer method is "setFred". Note that the property name should start with a lower case character, which will be capitalized in the method names.
Question number 3:
I agree with the suggestion of the software you're using. For readability, only one exit point is better. For efficiency, using 'return;' might be better. My guess is that the compiler is smart enough to always pick the efficient alternative and I'll bet that the bytecode would be the same in both cases.
FURTHER EMPIRICAL INFORMATION
I did some tests and found out that the java compiler I'm using (javac 1.5.0_19 on Mac OS X 10.4) is not applying the optimization I expected.
I used the following class to test:
public abstract class Test{
public int singleReturn(){
int ret = 0;
if (cond1())
ret = 1;
else if (cond2())
ret = 2;
else if (cond3())
ret = 3;
return ret;
}
public int multReturn(){
if (cond1()) return 1;
else if (cond2()) return 2;
else if (cond3()) return 3;
else return 0;
}
protected abstract boolean cond1();
protected abstract boolean cond2();
protected abstract boolean cond3();
}
Then, I analyzed the bytecode and found that for multReturn() there are several 'ireturn' statements, while there is only one for singleReturn(). Moreover, the bytecode of singleReturn() also includes several goto to the return statement.
I tested both methods with very simple implementations of cond1, cond2 and cond3. I made sure that the three conditions where equally provable. I found out a consistent difference in time of 3% to 6%, in favor of multReturn(). In this case, since the operations are very simple, the impact of the multiple return is quite noticeable.
Then I tested both methods using a more complicated implementation of cond1, cond2 and cond3, in order to make the impact of the different return less evident. I was shocked by the result! Now multReturn() is consistently slower than singleReturn() (between 2% and 3%). I don't know what is causing this difference because the rest of the code should be equal.
I think these unexpected results are caused by the JIT compiler of the JVM.
Anyway, I stand by my initial intuition: the compiler (or the JIT) can optimize these kind of things and this frees the developer to focus on writing code that is easily readable and maintainable.
Question number 6:
You could call a class method from your instance method and leave that static method alter the class variable.
Then, your code look similar to the following:
public static void clearInstance() {
instance = null;
}
@Override
public void handleEvent(Event e) {
switch (e.type) {
case SWT.Dispose: {
if (e.widget == getComposite()) {
MyClass.clearInstance();
}
break;
}
}
}
This would cause the warning you described in 5, but there has to be some compromise, and in this case it's just a smell, not an error.
Question number 7:
This is simply a smell of a possible problem. It's not necessarily bad or wrong, and you cannot be sure just by using this tool.
If you've got a real problem, like dependencies between constructors, testing should show it.
A different, but related, problem are circular dependencies between jars: while classes with circular dependencies can be compiled, circular dependencies between jars cannot be handled in the JVM because of the way class loaders work.
I have no idea. It seems likely that whatever you did do, it was not what you were attempting to do!
Perhaps the declarations appear in a Serializable class but that the type (e.g. ComboProgress are not themselves serializable). If this is UI code, then that seems very likely. I would merely comment the class to indicate that it should not be serialized.
This is a valid warning. You can refactor your code thus:
public void actionPerformedOnModifyComboLocations() {
if (!getMainTree().isFocusControl()) {
....//do stuffs, based on the initial test
}
}
This is why I can't stand static analysis tools. A null assignment obviously leaves you open to NullPointerExceptions later. However, there are plenty of places where this is simply unavoidable (e.g. using try catch finally to do resource cleanup using a Closeable)
This also seems like a valid warning and your use of static access would probably be considered a code smell by most developers. Consider refactoring via using dependency-injection to inject the resource-tracker into the classes where you use the static at the moment.
If your class has unused imports then these should be removed. This might make the warnings disappear. On the other hand, if the imports are required, you may have a genuine circular dependency, which is something like this:
class A {
private B b;
}
class B {
private A a;
}
This is usually a confusing state of affairs and leaves you open to an initialization problem. For example, you may accidentally add some code in the initialization of A that requires its B instance to be initialized. If you add similar code into B, then the circular dependency would mean that your code was actually broken (i.e. you couldn't construct either an A or a B.
Again an illustration of why I really don't like static analysis tools - they usually just provide you with a bunch of false positives. The circular-dependent code may work perfectly well and be extremely well-documented.
For point 3, probably the majority of developers these days would say the single-return rule is simply flat wrong, and on average leads to worse code. Others see that it a written-down rule, with historical credentials, some code that breaks it is hard to read, and so not following it is simply wrong.
You seem to agree with the first camp, but lack the confidence to tell the tool to turn off that rule.
The thing to remember is it is an easy rule to code in any checking tool, and some people do want it. So it is pretty much always implemented by them.
Whereas few (if any) enforce the more subjective 'guard; body; return calculation;' pattern that generally produces the easiest-to-read and simplest code.
So if you are looking at producing good code, rather than simply avoiding the worst code, that is one rule you probably do want to turn off.
发布评论
评论（3）
以下是我对您的一些问题的回答：
问题号2：
我认为您没有正确地对属性进行资本化。这些方法应称为 getPBar 和 setPBar。
JavaBeans 规范指出：
问题号3：
我同意您正在使用的软件的建议。为了可读性，只有一个出口点更好。为了提高效率，使用“return;”可能会更好。我的猜测是，编译器足够聪明，总是会选择有效的替代方案，并且我敢打赌，两种情况下的字节码都是相同的。
更多经验信息
我做了一些测试，发现我正在使用的 java 编译器（Mac OS X 10.4 上的 javac 1.5.0_19）没有应用我预期的优化。
我用下面的类来测试：
然后，我分析了字节码，发现对于 multReturn() 有多个 ireturn 语句，而 singleReturn() 只有一个。此外，singleReturn() 的字节码还包含几个返回语句的goto。
我使用 cond1、cond2 和 cond3 的非常简单的实现测试了这两种方法。我确保这三个条件同样可证明。我发现时间差异一致为 3% 到 6%，有利于 multReturn()。在这种情况下，由于操作非常简单，所以多重回报的影响是相当明显的。
然后我使用 cond1、cond2 和 cond3 的更复杂实现来测试这两种方法，以使不同返回的影响不那么明显。我对结果感到震惊！现在，multReturn() 始终慢于 singleReturn()（2% 到 3% 之间）。我不知道是什么导致了这种差异，因为代码的其余部分应该是相同的。
我认为这些意想不到的结果是JVM的JIT编译器造成的。
无论如何，我坚持我最初的直觉：编译器（或 JIT）可以优化此类事情，这使开发人员能够专注于编写易于阅读和维护的代码。
问题号6：
您可以从实例方法调用类方法，并让该静态方法更改类变量。
然后，您的代码看起来类似于以下内容：
这将导致您在 5 中描述的警告，但必须进行一些妥协，在这种情况下，它只是一种气味，而不是错误。
问题号7：
这只是一个可能存在问题的迹象。这不一定是坏事或错误，并且您不能仅通过使用此工具来确定。
如果您遇到真正的问题，例如构造函数之间的依赖关系，测试应该会显示出来。
另一个不同但相关的问题是 jar 之间的循环依赖：虽然可以编译具有循环依赖的类，但由于类加载器的工作方式，jar 之间的循环依赖无法在 JVM 中处理。
Here are my answers to some of your questions:
Question number 2:
I think you're not capitalizing the properties properly. The methods should be called getPBar and setPBar.
The JavaBeans specification states that:
Question number 3:
I agree with the suggestion of the software you're using. For readability, only one exit point is better. For efficiency, using 'return;' might be better. My guess is that the compiler is smart enough to always pick the efficient alternative and I'll bet that the bytecode would be the same in both cases.
FURTHER EMPIRICAL INFORMATION
I did some tests and found out that the java compiler I'm using (javac 1.5.0_19 on Mac OS X 10.4) is not applying the optimization I expected.
I used the following class to test:
Then, I analyzed the bytecode and found that for multReturn() there are several 'ireturn' statements, while there is only one for singleReturn(). Moreover, the bytecode of singleReturn() also includes several goto to the return statement.
I tested both methods with very simple implementations of cond1, cond2 and cond3. I made sure that the three conditions where equally provable. I found out a consistent difference in time of 3% to 6%, in favor of multReturn(). In this case, since the operations are very simple, the impact of the multiple return is quite noticeable.
Then I tested both methods using a more complicated implementation of cond1, cond2 and cond3, in order to make the impact of the different return less evident. I was shocked by the result! Now multReturn() is consistently slower than singleReturn() (between 2% and 3%). I don't know what is causing this difference because the rest of the code should be equal.
I think these unexpected results are caused by the JIT compiler of the JVM.
Anyway, I stand by my initial intuition: the compiler (or the JIT) can optimize these kind of things and this frees the developer to focus on writing code that is easily readable and maintainable.
Question number 6:
You could call a class method from your instance method and leave that static method alter the class variable.
Then, your code look similar to the following:
This would cause the warning you described in 5, but there has to be some compromise, and in this case it's just a smell, not an error.
Question number 7:
This is simply a smell of a possible problem. It's not necessarily bad or wrong, and you cannot be sure just by using this tool.
If you've got a real problem, like dependencies between constructors, testing should show it.
A different, but related, problem are circular dependencies between jars: while classes with circular dependencies can be compiled, circular dependencies between jars cannot be handled in the JVM because of the way class loaders work.
我不知道。看起来无论你做了什么，这都不是你想要做的！
也许声明出现在
Serialized类中，但该类型（例如ComboProgress本身不可序列化）。如果这是 UI 代码，那么这似乎很有可能。我只想评论该类以表明它不应该被序列化。这是一个有效的警告。您可以这样重构您的代码：
这就是为什么我不能忍受静态分析工具。显然，
null赋值会让您稍后面临NullPointerException。然而，有很多地方这是不可避免的（例如使用try catch finally使用Closeable进行资源清理）这似乎也是一个有效的警告大多数开发人员可能会认为您使用
静态访问是一种代码味道。考虑通过使用依赖注入进行重构，将资源跟踪器注入到您当前使用静态的类中。如果您的类有未使用导入，那么应该将其删除。这可能使警告消失。另一方面，如果需要导入，则可能存在真正的循环依赖，如下所示：
<前><代码>A类{
私人 B b；
}
B类{
私人Aa;
}
这通常是一种令人困惑的状态，并且会让您面临初始化问题。例如，您可能不小心在
A的初始化中添加了一些需要初始化其B实例的代码。如果您将类似的代码添加到B中，那么循环依赖将意味着您的代码实际上已损坏（即您无法构造A或B再次说明为什么我真的不喜欢静态分析工具 - 它们通常只是为您提供一堆误报，循环相关的代码可能工作得很好并且有很好的文档记录。
I have no idea. It seems likely that whatever you did do, it was not what you were attempting to do!
Perhaps the declarations appear in a
Serializableclass but that the type (e.g.ComboProgressare not themselves serializable). If this is UI code, then that seems very likely. I would merely comment the class to indicate that it should not be serialized.This is a valid warning. You can refactor your code thus:
This is why I can't stand static analysis tools. A
nullassignment obviously leaves you open toNullPointerExceptions later. However, there are plenty of places where this is simply unavoidable (e.g. usingtry catch finallyto do resource cleanup using aCloseable)This also seems like a valid warning and your use of
staticaccess would probably be considered a code smell by most developers. Consider refactoring via using dependency-injection to inject the resource-tracker into the classes where you use the static at the moment.If your class has unused imports then these should be removed. This might make the warnings disappear. On the other hand, if the imports are required, you may have a genuine circular dependency, which is something like this:
This is usually a confusing state of affairs and leaves you open to an initialization problem. For example, you may accidentally add some code in the initialization of
Athat requires itsBinstance to be initialized. If you add similar code intoB, then the circular dependency would mean that your code was actually broken (i.e. you couldn't construct either anAor aB.Again an illustration of why I really don't like static analysis tools - they usually just provide you with a bunch of false positives. The circular-dependent code may work perfectly well and be extremely well-documented.
对于第三点，现在大多数开发人员可能会说单返回规则完全是错误的，并且平均会导致更糟糕的代码。其他人则认为这是一条书面规则，具有历史依据，一些破坏它的代码很难阅读，因此不遵循它是完全错误的。
您似乎同意第一个阵营，但缺乏信心告诉工具关闭该规则。
要记住的是，在任何检查工具中编码都是一个简单的规则，而且有些人确实想要它。所以它几乎总是由他们实施。
然而很少（如果有的话）强制执行更主观的“保护”；身体;返回计算；'通常会生成最容易阅读和最简单的代码的模式。
因此，如果您希望生成好的代码，而不是简单地避免最差的代码，那么您可能确实想要关闭这一规则。
For point 3, probably the majority of developers these days would say the single-return rule is simply flat wrong, and on average leads to worse code. Others see that it a written-down rule, with historical credentials, some code that breaks it is hard to read, and so not following it is simply wrong.
You seem to agree with the first camp, but lack the confidence to tell the tool to turn off that rule.
The thing to remember is it is an easy rule to code in any checking tool, and some people do want it. So it is pretty much always implemented by them.
Whereas few (if any) enforce the more subjective 'guard; body; return calculation;' pattern that generally produces the easiest-to-read and simplest code.
So if you are looking at producing good code, rather than simply avoiding the worst code, that is one rule you probably do want to turn off.