This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
070730. 1. Create a new Java class NewClass. 2. Add private String foo; to the class. 3. Invoke Source - Insert Code - equals(). Include the foo field in the equals() method. Press Generate. Part of the generated code is if (this.foo != other.foo && (this.foo == null || !this.foo.equals(other.foo))) { for which the IDE displays a "Comparing Strings using == or !=" hint.
I do not see anything incorrect on the generated code itself. The main reason for the this.foo != other.foo expression is this.foo == null && other.foo == null detection.
Sure. But generating code which triggers a hint looks dumb.
P4 imo
The generated code would fail any serious code review and would be flagged by every static analysis tool.
*** Issue 118783 has been marked as a duplicate of this issue. ***
IMHO, hints in NB are like a spelling checker in a word processing program. They frequently catch mistakes, but not aways. In the case of the generated code, it is doing exactly what it should do - check the pointer to the object, not the contents of the object. However, this *looks like* a common code mistake - thus the hint. So, what to do about the editor hint? Let's borrow a solution from that word processing program. I suggest that NB allow me to "override" the editor hint. Let me mark the hint on this line so that this hint doesn't display any more. If I later change this line of code, the editor should throw away the exclusion and show me the hint again. This solution, to me, would be far more widely useful. It would let me turn off hints at the specific places where they are *not* helpful. Editors and static analysis tools can only go so far. It is us humans that must determine if the code is correct or not. If NB had this enhancement, then the generated code could generate the exclusion, too. I have changed this issue to an enhancement. If you disagree, please add more comments and change it again.
I think the generated code should be changed so that it does not use the == or != comparators. It looks silly for NetBeans to detect a problem and then to fix it by generating another problem. Even if this could be fixed so that NetBeans ignored the generated code, that would still not be good enough in my opinion. As soon as someone opened the same code in another IDE or ran automated tools like Checkstyle or FindBugs, it would flagged as well. To completely suppress it a developer would have to use a handful of suppression comments to make all of the tools happy. Then, after they got the tools happy they'd have to go to a code review and argue their case there as well. It's just not worth the hassle. Quick-fixes are supposed to save a developer time. This one does not.
Don't agree. The comparision improves performance and should not be thrown out because some tools warn against it.
I think it should just be left as is. If it is to ignore generated code then it would have to wrapped in some type of a comment. If someone using another IDE removes this comment the warning will appear in NB, and it would appear in some other IDE anyways. Then, if we tried to make it smarter...analyze string comparisons to try to detect this pattern then we just slow down the editor. I think the warnings are just part of it, but in this case we are talking about an artificial warning anyways. There are plenty of cases when the comparisons are perfectly valid. So, to me, the warning for strings is something which can be turned off, and probably should be turned off by default. Look, people new to Java probably aren't going to know they can not use == and != with their own classes. Someone who has been working with C/C++ for years...I would say shame on them ;-) should have known C/C++ == is just an override. Other languages...they will need to know this anyways. So, there are good arguments really for either way, but the main problem here is that a warning is being generated for code which the compiler won't be generating a warning, so it is already doing something outside the bounds of what it needs to do if you ask me as it ends up putting the development of something so small into such a context as it capitalizes on more time that it really needs just to generate a warning for something which is actually part of the language...generating a warning for comparing references. If anything is to be removed...I would argue the warning itself should just be removed all together, and if not, then this warning in this case should be lived with and maybe for this warning the warning message can have text appended to it explaining what it means in Java and that in some instances, but only for comparing references, it is OK, and maybe point the user to some specifications so they can learn the difference.
I understand the optimization, but also know that the String pool already takes care of this optimization. There is no performance gain with this code. I'm having flashbacks to tabs vs. spaces and placement of braces debates. :-) Josh/RationalPi
*** Issue 118767 has been marked as a duplicate of this issue. ***
The generated code is obviously OK. The only think I can do is to add something like: @SuppressWarnings("string-comparison-by-operator). What?
>>The generated code is obviously OK. The only think I can do is to add something like: >>@SuppressWarnings("string-comparison-by-operator). Please do that I suggest add small comment way this @SuppressWarnings("string-comparison-by-operator)
What? "I suggest add small comment way this @SuppressWarnings("string-comparison-by-operator)" I don't understand the sentence.
Sorry I suggest to add comment to generated method like @SuppressWarnings("string-comparison-by-operator")//why we add this
Pardon my French, but generating code which needs @SuppressWarnings() and then a comment explaining why that @SuppressWarnings() is there looks stupid. I also suppose "string-comparison-by-operator" would be a NetBeans-specific token, right? Yuck. I would suggest generating different code when String's would be compared. The generated code is nice and works fine for other types than String, so keep it. Just do it another way for String's.
I think that reference comparing should be removed because equals method should handle this, there is an unwritten(or not?) law that equals method should be null safe. if (this.foo == null || !this.foo.equals(other.foo)) { ... } may be some settings option for this?
If comparing primitives, it should use ==. If comparing Objects, it should use .equals(). Before using .equals(), it should check for null. It's silly to have to rewrite the same parts of a generated method every time. The following block is great for a short-circuit comparison of Object fields. public boolean equals(Object obj) { /* SNIP */ if ((this.foo == null) ? other.foo != null : !this.foo.equals(other.foo)) { return false; } return true; } For Object comparison, == is the exception, not the rule. If foo's .equals() method wants to perform a == comparison internally, then that's the best place for that optimization to happen: inside its own .equals() method. But you know, everyone is going to have a different preference. Why not just let the code template itself be editable? Then you can set it up however you want. I have filed Issue #142630 to that end.
Let's be clear though, there is no doubt that unless o == null (o.equals should throw an exception as you can't call methods on null references) then o.equals(o) should return true, and thus this argument is mute in itself because there is no need to make another method call to see whether o==o should end up being equal to o.equals(o). Thus: o.equals(o)&&(o==o)==true. and if this is ever not the case, then someone's code is wrong...there really isn't anything left to preference with this issue. At no point should o.equals(o) not return true in any circumstance which would make logical sense. Thus it is also true that if o==x, and x is an Object instance, they are references to the same instance, and a superfluous call to o.equals(x) is gong to return true, and the logic which uses == on the instance variables is thus correct and not wrong. This is the case with both local and remote instances. There should be no reason to rewrite logic which simply determines whether x==o on object instances as a basis for equality even with Objects. Obviously if Object o is the same instance as Object x they are equal. Now, the other way x!=o can not be correctly determined for object instances because the field values are not compared by this operation, as is the case with String and other classes, but if x==o then the field instances have to be equal as one can not change field y of instance x without changing field y in o when x==o. It is simply not possible in normal circumstances, and in the chance we're talking multi-processing, not using volatile can result in loss of information regardless of instance or method comparison. Also, I have no objection to this being configurable, but I do want the issue to be clear. The real issue is that the editor shows a hint to change something which is perfectly valid, and something which can keep the stack from being accessed unnecessarily. The stack and efficiency stuff is just icing as in almost all cases that little difference in speed and memory usage won't make a difference, but it is still a perfectly valid use of ==.
Of course a call to .equals() (equality test) can be avoided by testing for == (identity test) first. This is because the reflexive property of .equals() is a must. If x == y, then x.equals(y) is equivalent to x.equals(x). This short-circuit identity test is already done by the currently generated code. But there is nothing *wrong* with moving that short-circuit identity test to inside the .equals() method. In fact, you you look at the implementation for String.equals(), you'll see it's the first thing in there. Okay, now this has digressed into a philosophical discussion on proper OO design. Let's try to get back to the issue at hand. We already know that a != test by itself isn't sufficient to rule out equality, which is why it calls .equals(). But for comparing Strings specifically, a short-circuit call to != or == actually IS unnecessary. That's why I think the snippet above is ideal for String comparison. One obvious fix is to disable the warning about String comparison using ==. If you're okay with comparing Strings with == instead of .equals(), disable the hint: Tools > Options > Java Code > Hints > General > Comparing Strings using == or != Another option is to just have the .equals() code generation spit out different for comparing Strings which omits any == or != operations.
The point I'm making isn't about whether it is good OO design or not, this really doesn't have anything to do with OO as much as Java equality mechanisms, but simply that the code being produced is not incorrect, and giving the reason why it is correct regardless of preference. The short circuit simply isn't "incorrect" whether this becomes user configurable or not, and the fact that a hint is being generated for something which is perfectly fine, correct within the context used, and provides less memory and process overhead than a method call in the times the instances are the same, which must copy the reference and use the stack, is the thing I'm stressing.
Also, the short circuit works for other Object types besides String.
I'm sorry if I wasn't clear before. Let me reiterate. Using == to compare Strings is *always* unnecessary, since String.equals() performs a == test already. There is no reason to ever use == to compare Strings, and the hint exists to help you avoid it. Let me be clear about this too: the short-circuit == test to avoid a potentially expensive call to .equals() is perfectly fine when comparing any other kind of Object. The only time it's truly unnecessary is when comparing Strings. If you're that concerned about efficiency, consider this. Using the short-circuit == to compare Strings before invoking .equals() would actually be less efficient. In the case that they are not the same instance, the code would perform the == comparison twice, once for your external short-circuit, and once inside the String.equals() method. Why perform == externally when you know it is already performed by String.equals()? In short, when NetBeans generates code, it shouldn't then complain about the code it generates. There should be a special case for comparing String fields in a generated .equals() method, a case which avoids the redundant == short-circuit on Strings.
Please note that the main reason for the current code is not performance. The main intent of != is handling of situation when this.foo == null && other.foo == null. Any performance gains are purely accidental. The condition can be written in many ways (including the one proposed by malfunction84), but this one seemed to be the simplest/clearest to me. I do not think that we should compare Strings in a different way than other objects - if the test is good enough for any (and all) custom object(s), then it is good enough for String, IMO.
Fixed. Improper hint wont be shown anymore. --- http://hg.netbeans.org/main/rev/0c21dec795cf
I *really* don't think suppressing the hint inside .equals() is the best solution. The hint exists to remind the programmer that != and == do not perform String comparisons. Normally, if they make this mistake, they will see this very useful hint. Now, if they happen to make the mistake in a .equals() method, they will not see the hint. This is not consistent behavior. If the intent of using != in the generated code is to test for null, why not explicitly test for null? It will clarify the code, work for any type of Object (including String), and avoid using code which causes this hint to be displayed, while still allowing the hint to be displayed in the cases for which it was designed, even in a .equals() method.
We've discussed it long time before choosing this solution. Explicit null check will be necessary for every String field (or every variable, in case String wouldn't be treated specifically, as you suggested) included in equals -- which will enlarge amount of generated code. IMHO, everyone is able to comprehend the code currently generated. Besides, the pattern (inside equals() and inside logical AND tree) looks narrow enough to me. Any other opinions? Thanks.
I am following this discussion with a little smile on my face - it is one of the tiny (so, so tiny) problems in the universe... :) I will give it a shot - excuse me if I write nonsense (I am a lame programmer...): 0. Original statement: A & (B | C) this.foo != other.foo && (this.foo == null || !this.foo.equals(other.foo)) 1. Using 1st order predicate logics: (A & B) | (A & C) (this.foo != other.foo && this.foo == null) || (this.foo != other.foo && !this.foo.equals(other.foo)) > Explicit null check will be necessary for every String field (or every variable, in case String wouldn't be treated > specifically, as you suggested) included in equals -- **which will enlarge amount of generated code**. No, I don't think so - code that is equivalent does not need more formulas, see bellow: 2. First sub-statement of 1. can be rewritten with the same number of statement, as "null" is a logical constant: this.foo == null && other.foo != null 3. Second sub-statement of 1. can (don't have to be) be rewritten so that you omit the reference check (equals does not compare references but objects), but I don't know if leaving 2ns sub-statement unchanged does not gain some performance: !this.foo.equals(other.foo) Therefore, the equivalent statement (that is not triggering the warning) would be: (this.foo != null && other.foo == null) || (!this.foo.equals(other.foo)) [Note: this is equivalent to the ternary operator mentioned above (that I actually like quite a lot, as I did most of my code in C++/WinAPI), just in a little more human-friendly form] > Besides, the pattern (inside equals() and inside logical AND tree) looks narrow enough to me. I think code above is also well readable and narrow... Right? ;)
... sorry - the 3rd step would also require the null check on "this.foo"...
... and a typo in the resume... hmmm... nice...
joshis, the second clause in your statement would throw a NullPointerException if this.foo is null. The first clause would return false, but because it's ORed with the second, it would not short-circuit and the second clause would still execute, generating the NPE. Would you be willing to change the generated code for Object comparisons? I believe a simple change meets everyone's criteria. Current code: if (this.foo != other.foo && (this.foo == null || !this.foo.equals(other.foo))) Proposed code: if ((this.foo == null) ? (other.foo != null) : !this.foo.equals(other.foo)) This code is clear, has an explicit null check, avoids the warning if foo is a String, and uses fewer characters than the original code. It also allows you to undo the previous change which causes inconsistent behavior for the String comparison warning.
> joshis, the second clause in your statement would throw a NullPointerException if this.foo is null. This is why I added a comment bellow saying that the 3rd step needs a null check... > ... sorry - the 3rd step would also require the null check on "this.foo"...
> if ((this.foo == null) ? (other.foo != null) : !this.foo.equals(other.foo)) I personally like this - why not to give a renaissance to a ternary operator?
You're right. I'm sorry; I missed that part. So you're proposing: (this.foo != null && other.foo == null) || (this.foo != null && !this.foo.equals(other.foo)) Since both clauses contain "this.foo != null", by the distributive property, this can become: ((this.foo != null) && ((other.foo == null) || !this.foo.equals(other.foo)) Which is actually *very* similar to: ((this.foo == null) ? (other.foo != null) : !this.foo.equals(other.foo))
I made that confusing - sorry, I will cite myself again: >... and a typo in the resume... hmmm... nice. I made a typo in the first substatement of the resume (wrong negations) - but if you read the contents of the long comment ignoring the resume and comments bellow, you will see I proposed: (this.foo == null && other.foo != null) || (this.foo != null && !this.foo.equals(other.foo)) I also placed a note bellow the incorrectly concluded resume: > [Note: this is equivalent to the ternary operator mentioned above (that I actually like quite a lot, > as I did most of mycode in C++/WinAPI), just in a little more human-friendly form]
Integrated into 'main-golden', available in build *200808051401* on http://bits.netbeans.org/dev/nightly/ Changeset: http://hg.netbeans.org/main/rev/0c21dec795cf User: Max Sauer <msauer@netbeans.org> Log: #111441: [60cat]Generated equals() method uses != to compare strings
As long as we are solving issue "a warning is shown for auto generated code", I see two possible solutions: -change the generated code: e.g. by use the ternary operator (which seems quite strange in Java to me and I would guess it would be confusing for a lot of users), or by using some other proposed expression (considering only the valid ones, which are usually more cumbersome than one that is used currently). Moreover, the currently generated code is perfectly valid, to my knowledge, so this seems more like a workaround. -fix the hint to understand the generated expression and not issue the warning. I would strongly prefer this solution (even if the hint would be change to ignore this specific expression).
> fix the hint to understand the generated expression and not issue the warning. I would strongly prefer this solution > (even if the hint would be change to ignore this specific expression). I have personally no problem with this solution... > Moreover, the currently generated code is perfectly valid, to my knowledge, so this seems more like a workaround. Well, but imagine that you would not use the code completion and you would type equals method manually. Then you would see the hint in the IDE. You wouldn't try to avoid this warning? I know that the code is correct... but I don't like warnings in my code - what I would think: "There are cool Sun programmers that know Java perfectly and they are trying to tell me that this type of comparison I am making is not fully OK"... > cumbersome ??? (this.foo == null && other.foo != null) || (this.foo != null && !this.foo.equals(other.foo)) This literally says: (this.foo == null && other.foo != null) ~ If this.foo is null, is other.foo not null? If yes,return false. || ~ If the previous condition does not hold, check the one bellow... (this.foo != null && !this.foo.equals(other.foo)) ~ If this.foo is non-null, are the objects equal? If yes,return false. I will cite myself one more time: > I am following this discussion with a little smile on my face - it is one of the tiny (so, so tiny) problems in the > universe... :)
jlahoda, I agree that those are the only two solutions. We just happen to prefer different approaches. What code is better or makes more sense is completely subjective. We may all have different opinions about that. But objectively, causing the hint to behave differently based on where the code is will introduce inconsistency. Whether inconsistency is good or bad is subjective. I foresee this being brought up as a separate issue down the road ("No hint for 'Comparing Strings using == or !=' in .equals() method") because people will expect consistency. Someone might legitimately mean to use String.equals() in their .equals() implementation and accidentally use == or !=. With the most recent change (which was just merged into main-golden), they will not receive this hint. Please roll back the most recent change, as it introduces an inconsistency in how the hint is displayed. Let us address this issue in a way that does not compromise consistency. And to that end, I think we should change the generated code. The currently generated code is indeed perfectly valid, even for Strings. The reality is that if you're performing a == on a String and then perform a String.equals() on it in the same clause, it's probably fine. But the hint currently isn't smart enough to statically evaluate your code in that way. (Is this another option, perhaps?) I honestly believe we can agree on new code that is valid, clear, consistent, and won't generate this warning for Strings. Several proposals have been made already.
I am not proposing to disable the hint in the equals method. I am proposing to learn the hint that the (specific) condition is correct (which, honestly, is the most difficult solution that was proposed so far).
> which, honestly, is the most difficult solution that was proposed so far Indeed it is, because you need to teach/explain NB users why this specific condition is OK... I can see it in colors - issues saying "Reference comparison does not trigger warning" etc. I believe it is always a problem when the software is smarter than people who use it...
The revision which was merged into "main-golden" *does* disable the hint in .equals(). Sorry, I assumed you were involved with that change. I still believe that revision should be undone, due to the inconsistency it introduces. I'm not sure having the hint recognize specifically the generated code is a good idea either. It introduces the possibility that this issue will come up again if that code is ever changed, especially if that generated code is ever made to be editable (see Issue #142630). If the hint could recognize that a String.equals() is used in conjunction with a == or != comparison, that would be a more generalized, consistent solution that everyone might agree on. And it would also be, by far, the most difficult solution to implement. It would be difficult enough just to explain. I don't understand why you so adamantly oppose changing the generated code. I agree that the current code is valid, but so is alternative code which avoids a direct == or != comparison between the fields. Many of the commenters on this issue agree that the generated code should be changed. Strings are special, which is why the hint exists in the first place. If you oppose changing the generated code for all cases, please consider changing it for String comparisons.
Making the hint clever enough to recognize when the reference comparison is OK and when not is out-of-scope for this issue (and I would argue it is generally impossible). It is possible, though, to disable the hint altogether (and I am actually growing into supporting this option :-) ). Could you please list the advantages of changing the code (and keeping the hint as it is)? I see quite a few disadvantages: -currently existing code will still be marked by hint -if the user changes the code to the current one, it will still be marked by the hint (considering also issue #142630) -people that are satisfied with the current code will complain Also, I do not remember any other place people would be complaining about the code, only this issue. So it appears to me the main problem is that the generated code is marked with the hint. I strongly disagree with making the code for Strings more complex than for arbitrary user's object. We know much more about Strings than about user's objects. If we can use the knowledge to make the code simpler for Strings, that is OK. But I disagree with using the knowledge to make the code more complex.
All very good points. I agree that making the hint clever would be nigh-impossible. :) Before listing what I believe are the advantages, let me first address your disadvantages: > currently existing code will still be marked by hint Sure, and why not? The hint is there to discourage String comparison using ==. The problem is not that the hint *is* displayed (people do have the option to disable it), but that it is displayed on code that was just generated by the IDE. People who see this wonder to themselves, "Why doesn't the IDE consider its own hint and use different code?" > if the user changes the code to the current one, it will still be marked by the hint (considering also issue #142630)" Yes, as it should be (see above). This is consistent behavior for the hint, discouraging use of == on Strings anywhere. String.equals() already performs this test, so there's really no reason to do it unless testing for null. I don't see this as a disadvantage either. A user whose custom code does this should see a hint, too. > people that are satisfied with the current code will complain People are going to complain either way. I present exhibit A: myself. :) There is always the option to change the generated comparison for String fields only. And for the really hardcore complainers, there is always Issue #142630. There is a fourth disadvantage you mentioned, indirectly. > Making the code more complex for String comparison is bad. Simplicity does not always mean clarity. Assuming it would be more complex to handle Strings differently, why is this bad? Besides, how much complexity are we talking about, here? One more clause? We want the IDE to generate smart, usable code. I think most developers *would* and *do* handle String comparisons in an .equals() method differently than comparisons of other types. (Especially when a hint in their IDE suggests it!) But even so, I don't think this alternative approach for Strings would necessarily be any more complex than the currently generated code. Alright, the advantages, as I see them, to using a special comparison for Strings, or changing the base comparison code: - Allows more consistent behavior for the display of the hint. It can simply always be displayed, without the need for any fancy rules which users will not intuitively understand. - The code can be made simpler. Most of the proposed clauses use fewer characters than the current code. But even if there *are* more characters, that doesn't mean the code is more complex. - The code can be smarter. There's no reason why we shouldn't use the knowledge we have to make the IDE generate the best possible code. If this involves conditionally generating different code in different circumstances, that is a *good* thing for the user. - The code can be made clearer. The clause "this.foo != other.foo" does not look like a test for null at first glance, but it serves this purpose (as well as being a short-circuit). This is *less clear* than an explicit test for null. In contrast, my ternary proposal uses three very straightforward clauses, with parentheses to help make it readable. Or joshis's proposal, which includes explicit null checks and is really just an expansion of the ternary (for those of you who hate the ternary operator). Thanks for hearing me out. I hope I'm not frustrating you guys.
I have disabled the for currently generated condition: http://hg.netbeans.org/main/rev/0b3b94004d91 Lets see what will be the outcomes. I personally do not think there is a good reason to change the generated code. If you disagree, please feel free to file an enhancement (against java.editor).
I don't understand what your changes do. Can you explain? A separate issue would simply be a duplicate of this one. Recall the summary for this bug: "Generated equals() method uses != to compare strings." That really is the core of the problem. As we discussed before, there are two ways to resolve this issue: changing the hint or changing the generated code. Out of curiosity, I have gone back through the comments and tallied who is in favor of which solution. 5 - Change generated code for Strings (abadea*, deniss, joshis, malfunction84, rationalpi) 2 - Change how hint is applied (jlahoda**, msauer**) 2 - Allow developers to suppress hint on case-by-case basis (deeptinker, theanuradha) * Note that this is who originally filed the issue. ** Note that these are both NB developers. No other users have voiced their support for this solution. Based on this, I have changed the subcomponent on this bug to "editor" and reopened. The code which is generated for comparing Strings should be changed to avoid using !=, hint or no hint. For those who would like to change how comparisons of other types are generated, I have already filed Issue #142630. I will stress again, the hint was already working perfectly, so there was no reason to change it. The changes made in http://hg.netbeans.org/main/rev/0c21dec795cf should be reverted. Do you agree that they introduce an inconsistency? Do you agree that inconsistency should be avoided?
I pasted an incorrect link above. The correct one is: http://hg.netbeans.org/main?cmd=changeset;node=4f6c5daf99d6 (thanks to Max for notice) The hint simply checks whether the condition is the same as the condition generated by the equals generator. I still do not think the code should be changed (and, currently, I would be the one responsible of resolving any problems that would arise from such a change). If you can provide a patch and promise you will resolve any issues arising from the change, I will be happy to integrate it. Otherwise, please give me a very specific and very good reason to why change the code to a particular form that will be better than the current one (no references to the hint please - the code should be better than the current one itself, regardless of the hint). <rant>If we will change the code to read: ((this.foo == null) ? (other.foo != null) : !this.foo.equals(other.foo)), and someone will create a hint that will claim that using ternary operator inside if condition is not a good practice (which is true in my opinion), will you challenge the hint or the generated code?</rant> Changes from 0c21dec795cf do not exist anymore, to my knowledge.
Integrated into 'main-golden', available in build *200808071401* on http://bits.netbeans.org/dev/nightly/ Changeset: http://hg.netbeans.org/main/rev/4f6c5daf99d6 User: Jan Lahoda <jlahoda@netbeans.org> Log: #111441: disabling the "Incorrect String reference comparison" hint for the code that is autogenerated in the equals method.
The issue with String versus other Object doesn't matter in the context of the hint. The hint is there because some people falsely think == and != are the same as .equals and !.equals which would be incorrect. This rule applies to both String and non-String class types just the same. The hint isn't based on the fact that String just happens to have a is this==instance inside of its equal method, is it based on one not being able to use it for value comparison. Reference comparison is a perfectly legal and valid use. However, as someone mentioned, this has really become a mountain out of a mole hill type issue now, and it really is about preference at this point. In reality, tools, marking this as an error in all situations, is the real cause of the issue; that coupled with operator overloading in other languages, whether hidden from the user or not (C++). The shortest method to a resolution at this point, beyond repetitive back and forth, seems to be the best solution. I don't think constantly reopening and closing the issue is the best answer, and someone isn't going to be happy with any given solution, I think we just need to pick one which covers the situation, and is legal valid and working code which gets the job done.
Yes, 4f6c5daf99d6 has overwritten the changes introduced by 0c21dec795cf. jlahoda, your terms are acceptable. :) I will begin work on a patch. In regards to your rant, I would not challenge either. I believe there is a way to avoid pitting generated code against hints: simply have the code generation be aware of which hints are enabled. Bear with me, now. The hints represent developer code preferences, right? If we honor those preferences when generating code, we should never run into the situation where the IDE generates code which is marked with hints. And the hints will still be displayed in existing code as expected. So for instance, if the "Comparing Strings using == or !=" hint is enabled, we'll generate code that avoids using == or != to compare Strings. Otherwise, we'll use the default code. The same would be true of your ternary avoidance hint. If the hint is enabled, the code generation would not generate code which uses a ternary. I think this is actually a really good compromise that will please everyone. If no one objects, I will focus my efforts on implementing this.
(Not counting technical difficulties) I am not sure if creating code depending on enabled/disabled hints is a good idea: what if the user will generate some code and then enable a hint? Suddenly, his/hers code will be covered with warnings. Customizable code would be a better solution, IMO (but cannot be done for NB6.5, and a good default is needed anyway - most people will not change it).
I don't see a problem with that. That's why the warnings exist. It would be giving warnings on code that *was* generated, not warnings on code that *is being* generated. The IDE can't tell what code *was* generated, but it can tell what code *is being* generated because it's doing the generating. I don't know. I can see the hint setting acting more like a universal "treat Strings differently than other Objects when using == or != to compare" setting. It could control comparison warnings, code generation, and perhaps other behavior as well. And of course, you could always turn if off. Thoughts? Maybe that's a separate issue, literally. :P Anyway, for now, I will only work to change the generated code for String comparisons in a .equals() method. It won't be keyed off hint settings or anything. That has the smallest impact, and it's the simplest resolution.
Created attachment 68216 [details] patch - proposed solution
Thanks for the patch, it seems fine - I will integrate it with your name if you agree. BTW: I still do not understand what is so special about j.l.String (compared to e.g. j.l.Integer) that we generate a special code for it, but that is your problem now :-).
Yes, please integrate the patch with my name. This also means you can revert 4f6c5daf99d6 since it no longer generates that code for Strings. But yeah, I understand your point. I consider String to be different because (1) its .equals() method is different and (2) Java treats Strings differently than other other languages, particularly PHP. The .equals() methods for the primitive wrapper classes don't internally perform a == comparison. This means the two types of comparison are exclusive. For primitive wrappers (as with most other objects), .equals() is for comparing the values that instances contain, whereas == is used to discriminate between instances representing the same value. With Strings, .equals() performs a == comparison internally, so using .equals() is sufficient by itself for both comparing represented values and discriminating between instances. This renders == unnecessary. That and the fact that those new to Java use it incorrectly (the reason the warning exists) suggests to me that Strings need special consideration. I say this only to summarize my position for the sake of anyone who comes into this new, not to reignite the debate. Hooray. I get to field all objections.
http://hg.netbeans.org/main?cmd=changeset;node=baf21a21ca77
Integrated into 'main-golden', available in build *200808280201* on http://bits.netbeans.org/dev/nightly/ Changeset: http://hg.netbeans.org/main/rev/baf21a21ca77 User: Mark Lewis <malfunction84@netbeans.org> Log: #111441: [60cat]Generated equals() method uses != to compare strings