Community
Participate
Working Groups
If I create a new method via content assists I always delete the generated TODOs. I talked to several other developers about it and they see don't see this as an added value. I think we should generate code as efficient for the developer as possible. As the additional deletion step creates additional work, I suggest to remove that from the default templates. Someone who really like that can still add this. Here is an example a generated inner class: button.addControlListener(new ControlListener() { @Override public void controlResized(ControlEvent e) { // TODO Auto-generated method stub } @Override public void controlMoved(ControlEvent e) { // TODO Auto-generated method stub } });
As a Java developer i find this functionality very useful especially while working on huge files. 'TODO' is a task tag which gives a blue marker on the gutter and so it is easier to find where this new method was added. Lars, i agree with you that for small files it might not add much value, but when we look at the bigger picture i would like to keep it the way it is now.
How about, instead of a method with TODO like this: public static void controlMoved() { // TODO Auto-generated method stub } produce public static void controlMoved() { ; } Now instead of a TODO that could easily be forgotten this version gives something better: a compiler warning and the beginnings of the implementation, all-in-one!
(In reply to Timo Kinnunen from comment #2) > How about, instead of a method with TODO like this: > public static void controlMoved() { > // TODO Auto-generated method stub > } > produce > public static void controlMoved() { > ; > } > > Now instead of a TODO that could easily be forgotten this version gives > something better: a compiler warning and the beginnings of the > implementation, all-in-one! -1 for additional compiler warnings. If he have to choose between TODO or compiler warning I go for a TODO.
(In reply to comment #3) > -1 for additional compiler warnings. If he have to choose between TODO or > compiler warning I go for a TODO. Hahaha, but this is exactly why you should pick the compiler warning out of the two, because it feels more uncomfortable :) A method stub which hasn't been implemented yet and a method whose implementation consists of one lone semicolon like above are the same thing in the end, after all. And if I may muse a bit, having any of // FIXME can throw NPEs sometimes // TODO optimize this feature // XXX is this correct? implies defects, deficiencies and uncertainties about the code, so I'd say a warning can and should be issued here as well.
(In reply to Timo Kinnunen from comment #4) > (In reply to comment #3) > > > -1 for additional compiler warnings. If he have to choose between TODO or > > compiler warning I go for a TODO. > > Hahaha, but this is exactly why you should pick the compiler warning out of > the two, because it feels more uncomfortable :) A method stub which hasn't > been implemented yet and a method whose implementation consists of one lone > semicolon like above are the same thing in the end, after all. > > And if I may muse a bit, having any of > // FIXME can throw NPEs sometimes > // TODO optimize this feature > // XXX is this correct? > > implies defects, deficiencies and uncertainties about the code, so I'd say a > warning can and should be issued here as well. Note that those who actually prefer the warning or error over the task can easily achieve that by changing the template to be empty or contain the ';' plus setting the corresponding compiler options. I agree with Manju that we'll leave this as is.
I discussed this issue with Noopur on Devoxx. IMHO especially the //TODO for anonymous inner classes forces developers to use code completion and afterwards delete the generated TODO line. Please also note that IntelliJ does not generate such statements. I had the impression that Noopor agreed that this is not a good pattern for simple cases like anonymous inner classes and that she was open for a new discussion. Reopening so Noopur can have the discussion with her team.
We had a discussion on this in the team. Lars, could you please list down the cases where you feel that the generated TODO comment can be omitted by default? We can consider those cases to see if it would be fine to omit the default TODO comment there.
(In reply to Noopur Gupta from comment #7) > We can consider those cases to see if it would be fine to omit the default > TODO comment there. Great news. I'm currently at a customer side, hence my first short answer would be: 1.) Anonymous inner classes I update the bug once I have time to test the standard code templates from Eclipse again (for myself I remove the //TODO's from the templates whenever I see them)
I suggest to remove the //TODO code templates from the following entries in the preferences: - Method body - Constructor body For catch blocks I think it is OK to leave the //TODO Also for Comments I suggest to replace the /* (non-Javadoc) * ${see_to_overridden} */ with /** * */
(In reply to Lars Vogel from comment #8) > 1.) Anonymous inner classes Where do you find this? (In reply to Lars Vogel from comment #9) > I suggest to remove the //TODO code templates from the following entries in > the preferences: > > - Method body > - Constructor body > > For catch blocks I think it is OK to leave the //TODO Fine with me.
(In reply to Dani Megert from comment #10) > Where do you find this? Ignore, this was from memory. > (In reply to Lars Vogel from comment #9) > > I suggest to remove the //TODO code templates from the following entries in > > the preferences: > > > > - Method body > > - Constructor body > > > > For catch blocks I think it is OK to leave the //TODO > > Fine with me. Great news!
Currently we generate an empty line in the body, e.g. private void foo() { // TODO Auto-generated method stub } I suggest to keep that empty line: private void foo() { }
(In reply to Dani Megert from comment #12) > Currently we generate an empty line in the body, e.g. > > private void foo() { > // TODO Auto-generated method stub > > } > > I suggest to keep that empty line: > > private void foo() { > > } This won't work without code changes. Currently, if a code template only contains whitespace it will be trimmed and we get private void foo() { }
Removing TODO comments in Code Templates preferences for method body and constructor body will remove these comments in all the cases where method and constructor stubs are generated based on these templates. This is fine for simple cases like content assist and quick fix/assist where the stubs are created in front of the user. But it also affects refactorings where the stubs could be created in multiple files. In these cases, TODO comments act as an indicator of the changed code in all the files, which is useful in my opinion. Though we have previews for these refactorings, the user will still have to remember all the locations where the method stubs are created.
(In reply to Noopur Gupta from comment #14) > Removing TODO comments in Code Templates preferences for method body and > constructor body will remove these comments in all the cases where method > and constructor stubs are generated based on these templates. > > This is fine for simple cases like content assist and quick fix/assist where > the stubs are created in front of the user. > > But it also affects refactorings where the stubs could be created in > multiple files. In these cases, TODO comments act as an indicator of the > changed code in all the files, which is useful in my opinion. Though we have > previews for these refactorings, the user will still have to remember all > the locations where the method stubs are created. Good point. One solution would be to add new code templates, so that we and the user can choose different templates for the different cases.
(In reply to Noopur Gupta from comment #14) > But it also affects refactorings where the stubs could be created in > multiple files. Which refactorings result in method stubs which have TODO included? I'm only aware of the (non-Javadoc) one for extracting interfaces. If that is the only one, we could skip it for the first step.
(In reply to Lars Vogel from comment #16) > (In reply to Noopur Gupta from comment #14) > > > But it also affects refactorings where the stubs could be created in > > multiple files. > > Which refactorings result in method stubs which have TODO included? I'm only > aware of the (non-Javadoc) one for extracting interfaces. If that is the > only one, we could skip it for the first step. You can remove the TODO from default-codetemplates.xml and run the JDT UI tests to see some examples like org.eclipse.jdt.ui.tests.refactoring.PullUpTests.test52().
