Community
Participate
Working Groups
AFAIK Boolean.FALSE.equals(statement) is the preferred way of comparing the value of an existing string.
I think would be better to use Boolean.valueOf( stringVariable ) Indeed, using this is not good "false".equals( stringVariable ) but the one below compares a boolean with the Object variable2. Boolean.FALSE.equals( objectVariable ) And the Booolean.equals(Object) API says: Returns true if and only if the argument is not null and is a Boolean object that represents the same boolean value as this object.
Patrik, can you take this one?
I decided to resolve this by replacing occurrences of: "false".equals "true".equals with Boolean.FALSE.toString().equals Boolean.TRUE.toString().equals I'll provide different changes for different repositories.
New Gerrit change created: https://git.eclipse.org/r/76271
New Gerrit change created: https://git.eclipse.org/r/76272
Sorry, don't get me wrong, but what is the reason for the changes? I've read through the proposed patch, but the readability is much worse (more and more complicated, "strange" code to read), performance wise it is also not better in any way.
(In reply to Andrey Loskutov from comment #6) > Sorry, don't get me wrong, but what is the reason for the changes? > > I've read through the proposed patch, but the readability is much worse > (more and more complicated, "strange" code to read), performance wise it is > also not better in any way. Many thanks for yor precious feedback! I think the point is: using strings "true" or "false" is error-prone Ideally, we should use a String constant or a static method, instead of using a string. Initially I considered using the Boolean#valueOf(String s). But this works does not work for all cases; e.g., in this case: if ("true".equals(arg)) { return Boolean.TRUE; } else if ("false".equals(arg)) { return Boolean.FALSE; } else { // Other } The possible replacement is unclear (1) and error prone (2) if ( Boolean.valueOf(arg)) { //(1) return Boolean.TRUE; } else if (!Boolean.valueOf(arg)) { //(1) return Boolean.FALSE; } else { // Other // (2) in this case you'll never enter this block } So, I decided to use the Boolean.FALSE.toString(), as - Boolean.FALSE is static final - and toString looks not too bad in terms of performances. public String toString() { return value ? "true" : "false"; } Another option, is using a String constant, but I don't want to define a global constant in Eclipse, and I think we don't have a Java variable like String.TRUE or String.FALSE. Any suggestion is welcome!
(In reply to Patrik Suzzi from comment #7) > (In reply to Andrey Loskutov from comment #6) > > Sorry, don't get me wrong, but what is the reason for the changes? > > > > I've read through the proposed patch, but the readability is much worse > > (more and more complicated, "strange" code to read), performance wise it is > > also not better in any way. > > Many thanks for yor precious feedback! > > I think the point is: using strings "true" or "false" is error-prone All the examples below are more error prone as "true".equals(Object); which is short, clear and fast. Simply compare how many hops with your eays your need to read the code below: "true".equals(Object); vs Boolean.TRUE.toString().equals(Object); especially the second line will be wrapped to multiple in most cases. Regarding performance: "true".equals(Object): LDC "true" ALOAD 1: o INVOKEVIRTUAL String.equals (Object) : boolean POP Boolean.TRUE.toString().equals(Object): GETSTATIC Boolean.TRUE : Boolean INVOKEVIRTUAL Boolean.toString () : String ALOAD 1: o INVOKEVIRTUAL String.equals (Object) : boolean POP + Boolean.toString() public toString() : String L0 LINENUMBER 189 L0 ALOAD 0: this GETFIELD Boolean.value : boolean IFEQ L1 LDC "true" GOTO L2 L1 LDC "false" L2 ARETURN Sure this can be optimized, BUT usually this UI code in question is not running as often as to be optimized.
(In reply to Andrey Loskutov from comment #8) > Regarding performance: > > "true".equals(Object): > > LDC "true" > ALOAD 1: o > INVOKEVIRTUAL String.equals (Object) : boolean > POP > > Boolean.TRUE.toString().equals(Object): > > GETSTATIC Boolean.TRUE : Boolean > INVOKEVIRTUAL Boolean.toString () : String > ALOAD 1: o > INVOKEVIRTUAL String.equals (Object) : boolean > POP > > + Boolean.toString() > > public toString() : String > L0 > LINENUMBER 189 L0 > ALOAD 0: this > GETFIELD Boolean.value : boolean > IFEQ L1 > LDC "true" > GOTO L2 > L1 > LDC "false" > L2 > ARETURN > Didn't know that. I suggest to close my bug as wontfix in this csae.
I totally agree with you: Closing as wontfix Thanks for the feedback! It's my fault: I just tried to have this bug solved, I did not realize that I should have asked in advance.
