Annoying default editor inspections

My disk ran out of space the other day, and IDEA couldn't store its config
files, so I lost most of my IDEA settings. I reset my enabled editor inspections
to the defaults that now ship with Irida, and I've been trying to use them
to see what it's like for new users.

So far a few of them have annoyed me to the point that I think they should
not be enabled by default:

- Duplicate string constant. Maybe in some applications this is a serious
issue, but for me, it's not, and it happens all the time, almost exclusively
for strings which are conceptually different and could not logically be extracted
into a constant field.

- Collection is never updated or accessed. This one is totally silly in a
lot of common cases. Obviously when I first declare a collection field, it
hasn't been used yet. I don't need a computer to tell me this.

- Field can be converted to one or more local variables. Again, if I'm making
a new class, and I'm writing the first method in the class, then obviously
the fields it uses could be converted to local variables.

- Infinite loop statement. Same as above, when I'm editing code, I know that
the loop won't end, because I haven't gotten a chance to type "break" yet.

Another which only annoys me somewhat:

- Empty catch block. Sometimes I use descriptive exception variable names
to explain empty catch blocks:
} catch (IOException ignored) { }

What do others think about these? Do these bother you?


A side note, I think the "(J2SDK5.0 Only)" is ugly and unnecessary in the
name of, for example, "unnecessary boxing (J2SDK5.0 only)".


0
17 comments

- Duplicate string constant. Maybe in some
applications this is a serious
issue, but for me, it's not, and it happens all the
time, almost exclusively
for strings which are conceptually different and
could not logically be extracted
into a constant field.


Yes, have found it annoying.

- Collection is never updated or accessed. This one
is totally silly in a
lot of common cases. Obviously when I first declare a
collection field, it
hasn't been used yet. I don't need a computer to tell
me this.


I also think the analysis on this needs to get smarter. This inspection doesn't consider assignment of the Collection such as from a parameter in the constructor. Yes, literally speaking the contents are never updated other than the assignment to a likely non-empty Collection parameter, but shouldn't that be enough?

- Empty catch block. Sometimes I use descriptive
exception variable names
to explain empty catch blocks:
} catch (IOException ignored) { }


Agreed as I do the same. It would be good if a filter could be added to ignore specified exception variable names.
http://www.jetbrains.net/jira/browse/IDEA-1528

I would also add that although no single inspection seems to kill performance, IDEA seems pretty slow with as many inspections on as included with the Default.

Jon

0

These ones I'd really like to have by default:
- Collection is never updated or accessed.
- Field can be converted to one or more local variables.
- Infinite loop statement.
Off course they trigger until I finished the code, but that's the same for many other inspections (e.g. unused assignment). I really like it because it prevents me from forgetting to finish the code. E.g. before using Idea I often wrote a listener class and never registered it - which I only ever noticed after compilation, starting, and testing the program.
Now Idea saves me a lot of time by alerting me to my stupidity...

0


>Duplicate string constant.

Max's suggestion, and I can see it being a problem. I run with it off myself, for just the reasons you describe.

>Collection is never updated or accessed. This one is totally silly in a
>lot of common cases. Obviously when I first declare a collection field, it
>hasn't been used yet. I don't need a computer to tell me this.

I disagree. I very much like having my computer tell me when I haven't finished something. It may be annoying when I start a task, but it's great later on when I'm halfway through, or keeping changes from regressing me to a sub-optimal state. I consider this inspection pair to the built-in "variable is assigned but not used" and, to be honest, it's the inspection of which I'm most proud of discovering implementing. De Gustibus.

>Field can be converted to one or more local variables

Not one of mine, and it's too soon for me to say, but I think the "tell me that the task is unfinished" logic holds here.

>Infinite loop statement
Another "tell me that the task is unfinished" inspection. I've simply never had it trigger in the two years since I implemented it, so I can't comment on it's annoyance/value ratio.

>A side note, I think the "(J2SDK5.0 Only)" is ugly and unnecessary in the
>name of, for example, "unnecessary boxing (J2SDK5.0 only)".

Yup, me too. Someone on the JetBrains doc team made that change.

--Dave Griffith

0


>I also think the analysis on this needs to get smarter. >This inspection doesn't consider assignment of the
>Collection such as from a parameter in the constructor.

Bug. Just fixed it yesterday.

>It would be good if a filter could be added to ignore specified exception variable names.

Great idea, and fits a common coding style. I'll whomp it up on the train ride this morning for both "Empty Catch Block" and "Unused Catch Parameter".

--Dave Griffith

0

As for the 3 "tell me the task is not finished" inspections discussed here, would it be too much for those to have some longer delay when before inspecting code as you write it?

0

I disagree. I very much like having my computer tell me when I
haven't finished something. It may be annoying when I start a task,
but it's great later on when I'm halfway through, or keeping changes
from regressing me to a sub-optimal state. I consider this inspection
pair to the built-in "variable is assigned but not used" and, to be
honest, it's the inspection of which I'm most proud of discovering
implementing. De Gustibus.


Okay, but could you make it wait until the field has actually been used?
I keep getting it highlighted along with "field xxx is never used", which
makes the collection warning redundant.



0

I think this would be cool. I don't quite know how it could be designed,
for maximum usability and intuition. Maybe the infinite loop inspection could
only highlight if there are statements anywhere after it in the method, or
if the cursor is in another method, for example. Maybe the "field could be
local" should only show up if the cursor is in a method which does not use
the field, or something like that. What do you think about that, Dave?

As for the 3 "tell me the task is not finished" inspections discussed
here, would it be too much for those to have some longer delay when
before inspecting code as you write it?




0

"too much for those to have some longer delay when before inspecting code as you write it?"???
Sorry, I guess my internal parser choked on this...
My buest answer would be something like
"It would be ok if a little less than before when reading the inspection as I meant it to have in code some errors."

0

That's because you should have delayed language inspection until I've written my question correctly, :) but alas I was too lazy to clean it up first before when...

0


Good one, and trivial to implement. It'll be in the next EAP.

--Dave Griffith

0

>Maybe the infinite loop inspection could
only highlight if there are statements anywhere after it in the method

In that case, you should already be getting a red compiler warning for unreachable code.

--Dave Griffith

0

I would also add that although no single inspection seems
to kill performance, IDEA seems pretty slow with as many
inspections on as included with the Default.


I blamed the slow performance of recent EAP builds on the inspections as well, but when I turned them ALL off, Idea was just as slow for me. Probably just the usual EAP slowness.

0

While we're making suggestions for the inspections, it would be nice to have a search feature in the inspections dialog. I can never find what I'm looking for.

And a suggestion for a new inspection: "replaceAll() result not assigned".

I regularly run into bugs like this:
String s = "Hi Robert";
s.replaceAll("Robert", "Bob");

it should be this:
String s = "Hi Robert";
s = s.replaceAll("Robert", "Bob");

0

>And a suggestion for a new inspection: "replaceAll() result not assigned".

This is already covered by "Result of method call ignored". Any call to a String object where the result is not used will be flagged, if you enable that inspection.

Search feature seems to be in Irida. Haven't used it yet.

--Dave Griffith

0

Search feature seems to be in Irida. Haven't used it
yet.


I have and it's absolutely brilliant. It has saved me lots of time already. Love it.

Bas

0

What search feature you are talking about? Structural search?

Tom

0

No, just the search filter in the inspections dialog.

Bas

0

Please sign in to leave a comment.