Several New Hints

Hi all! Today we would like to introduce you some of our new experimental hints for NetBeans 7.2. They are called: Unused Use Statement and Immutable Variables.

Unused Use Statement

This hint is quite simple. It highlights (underlines) your use statements, which are not used.

Unused Use Statement

Typical use case is after some refactoring, when you forgot to remove some obsolete use statements. This hint warns you on them and allows you to remove them easily. Just click on the hint bulb in the gutter and select Remove Unused Use Statement.

Unused Use Statement

And of course, it works in multiple use statements combined too.

Unused Use Statement

Immutable Variables

The next one is the hint which checks too many assignments into a variable. And why? That's simple. Mostly you should use just one assignment into one variable. But sometimes you are lazy and you do something like:

Immutable Variables

But it's quite wrong, because what you really do is:

Immutable Variables

And that's exactly the case, when our new hint warns you, that Too many assignments (2) into variable $foo occured. Nothing more.

Immutable Variables

Yes, we know that there are some cases, where could be more assignments and no warning should occur, e.g.:

Immutable Variables

Because maybe one likes longer increment syntax more than the short one. So we tried to handle these cases to don't bother you if it's not a need.

Note: We are almost sure that this hint doesn't cover all your use cases, because there are a lot of them. So if you find something strange, write it into our bugzilla so we can handle it better for you. Thanks for your patience!

And the last thing is, that you can set the number of allowed assignments in Tools -> Options -> Editor -> Hints -> PHP: Immutable Variables.

Immutable Variables

Note: This hint works just for a common variables, not for fields. We have an enhancement request for that and it should be implemented in next version of NetBeans (probably 7.3).

And that's all for today and as usual, please test it and if you find something strange, don't hesitate to file a new issue (product php, component Editor). Thanks.

Comments:

Great! Thanks for these features!

Posted by pulzarraider on April 11, 2012 at 03:27 PM CEST #

I particularly appreciate the automation of the use statement. That's a great leap forward, considering that a few versions back, Netbeans wasn't supporting namespaces at all. You guys do a good job keeping up with new stuff.

If the reverse get done, that is automatically importing classes that are used in the class (as is done by Netbeans with Java), we won't have to care about maintaining use statements at all :)

Posted by Éric O. on April 12, 2012 at 01:25 AM CEST #

Unfortunately the "Immutable Variables" Hint was the first I turned off immediately. The thought is nice, but in practice it didn't work.

Junior developers decided, instead of making:

$foo = " bar ",
$fooTrimmed = trim($foo);
$fooWithoutDigits = preg_replace($regEx, $replace, $fooTrimmed);

The hint just made the devs either change it to:

$foo = preg_replace($regEx, $replace, trim(" bar "));

Or do something like this:

$foo = " bar ";
$foo2 = trim($foo);
$foo3 = preg_replace($regEx, $replace, $foo2);

I think the hint-text is not good. It simply reads "Too many assignments". But it does not explain why this is a problem and how to solve it. The explanation should be better AND the default value should be raised to 2 instead of 1.

Some examples:

$foo = null;
if ($test) {
$foo = 1;
} else {
$foo = 2;
}

// do some foo

In this case the hint causes developers to think they should drop the variable initialization.

$foo = 0;

if ($test) {
$foo = 1;
} elseif ($test2) {
$foo = 2;
}

return $foo;

In this case the hint caused developers to use multiple Return-statements instead.

I think that Junior devs are brilliant in coming up with ways on how to NOT do something right. So this is the one hint that needs more explanation than it currently does.

Please: raise the default value to "2" and EITHER deactivate it by default, OR improve the hint text (possibly by adding a link to a tutorial).

Posted by Tom on April 12, 2012 at 04:16 AM CEST #

Tom: Thanks for your reply!

The hint says what it really does...it checks a number of assignments.

Your examples are fixed in trunk (these cases are that special cases I was talking about in a blog post ;), no warning occurs even though you have your default value of assignments set to 1.

I think that this strict apporach (1 assignment) is good. If someone doesn't agree with that, there is a possibility to change it in settings, so I don't see any problem.

The real problems are that "special use cases", e.g. that one you mentioned - which is fixed. If you have some other, provide it to me so I can improve an implementation.

And the naming convention of "extra variables"...$foo, $foo1...it's developers responsibility. Developer should know, that the variable name should be self-explanatory. I can't do anything with that, if someone consider to use "numbered" variables...it's his choise. Unfortunately.

Posted by Ondrej Brejla on April 12, 2012 at 05:17 AM CEST #

I do not think this is a proper hint because it conflicts with another best practice of initializing a variable before using it, which the hint does not appear to be working correctly (will update to new nightly). In any case, your code does not catch the exception of initializing outside of a try... catch block and the hint is quite annoying.

It does force me to reevaluate my code, but on second or third glance, I find that no, the initialized variable is a better practice than removing it and the hint is indeed wrong for this instance. Quite annoying.

And refreshing. I do wish more code smells could be integrated into the IDE, it would be easier than simply using a tool that can easily be updated out of band of the IDE... A plugin! That would work well.

Posted by Jacob Santos on April 12, 2012 at 02:26 PM CEST #

@Ondrej Don't get me wrong: I appreciate the new hint. It's just that I hope you keep us smaller companies in mind.

Here we got 1 Senior developer that has to take care of 4 Junior developers, that all work overtime already and we still have enough work for 4 more developers that all was due yesterday.

These Junior devs find it hard to even understand what an interface is or why it is "bad" to program using copy&paste.

We would love to hire more senior developers, but the job-market is depleted. It's hard enough to get anybody at all.
Thus we have to stick with what we get, while there is hardly enough time for at least the minimal training that these guys deserve.

A hint thus needs to be self-explanatory to be useful. If all it creates is confusion with the Junior developers, then I as the Senior will have to invest company time explaining stuff to them.
And while I would love to do, our boss is slightly less enthusiastic on this idea.

So please: add an explanatory documentation to this hint on WHY it is bad and HOW to best solve the issue.
Just keep in mind: Junior developers are fresh meat. They might think that they are professionals, but in practice they know next to nothing just yet.

Thus: if it may confuse the youngsters, make it optional.
Make my life easier.

BTW: this is how I tell my Junior devs to avoid the issue.

assert(!isset($foo));
$foo = "";

// do some foo

unset($foo);

Once this is done, they may reassign $foo all they want.
Since the real problem here is not multiple assignment but accidental assignment, because they thought the variable was not in use.

If you stick with the code above it will work in any editor, even when you don't use or care for the code hint.

Posted by Tom on April 13, 2012 at 09:12 AM CEST #

I really like the unused use statement hint, thanks! There are a few too many use cases for the 'too many assignments' hint to work properly, so I will most likely turn it off or set it to 'show as Info'. Still a good addition to the hints list.

Posted by Pieter on April 13, 2012 at 07:55 PM CEST #

I see a general problem of basic assumptions here.

I still don't believe that "too many assignments" is enough of an explanation.

I suggest to rename the text "too many assignments" to something that makes the purpose more clear like: "You should use only 1 assignment to a variable to avoid accidentally overwriting it and make your code easier to read".

It seems you as a senior developer expect about the same level of understanding on the junior-dev's side.

THIS ASSUMPTION IS JUST DEAD WRONG!
You are a lot closer to reality if you expect them to believe that an SQL injection is a another type of flu-shot. (I'm exaggerating a bit, but not much: believe me!)

Don't just expect a junior-dev to understand what you mean by something you say (or the IDE says).
As a senior-dev (and as a computer program) you have to EXPLAIN it to people. Using W.W.H.: WHAT is wrong, WHY it is wrong and HOW to fix it.

If it wasn't that way we would all be seniors and there would be no juniors anymore.

Otherwise it won't help but just make things worse. Because when juniors don't understand what is expected and why, they will do "something" that satisfies the rules ... usually while raising the WTF-factor of the code.

The best software is worth nothing if the user doesn't understand it's features and thus is unable to put them to best use!

Ergo: Usability is an essential part of software quality.

You have done a better job on other hints Like "You should use === instead of == to have better control over your code".
It's still not perfect as "better control" is not explained, but it states: WHAT is wrong, WHY it is wrong and HOW to solve it.

So for good documentation always remember the WWH (WHAT, WHY and HOW).
Keep that pattern in mind and try to put it to best use.

Posted by Tom on April 22, 2012 at 04:45 PM CEST #

Hi Tom,

thanks for this discussion...and I have to agree with you. Your "hint description" is really better ("You should use only 1 assignment to a variable to avoid accidentally overwriting it and make your code easier to read"), so I'll try to fix it for 7.2.

Thanks!

Posted by Ondrej Brejla on April 23, 2012 at 09:52 AM CEST #

So it's there: http://hg.netbeans.org/web-main/rev/51eb4e93cf7a

Thanks!

Posted by Ondrej Brejla on April 23, 2012 at 11:07 AM CEST #

The "You should use only 1 assignment to a variable to avoid accidentally overwriting it and make your code easier to read" is just driving me mad, as it also shows up when doing something like

$foo = initializeFoo();

$foo = processInitializedFoo($foo);

The second assignment depends on the first, so the warning should not show up in these cases.

Posted by LT on August 03, 2012 at 03:10 PM CEST #

The "immutable variable" hint is nearly useless. It might fix only a very select few of the many possible bugs one could create. Plus as many have already said, it encourages writing unreadable code.

Quoting someone else on this:

"Constants are immutable, read-only properties are immutable, variables are mutable. That is why they are called vari-able"
http://chat.stackoverflow.com/transcript/message/4846750#4846750

Posted by guest on August 08, 2012 at 11:29 AM CEST #

Post a Comment:
  • HTML Syntax: NOT allowed
About

This blogs is written by NetBeans developers who contribute to the PHP support mainly.

Search

Archives
« April 2014
SunMonTueWedThuFriSat
  
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
   
       
Today