Community
Participate
Working Groups
We need to make it easy to create a main method for students so that they can move easily between their first programs built entirely in the main method to slightly more advanced classes in the context of object oriented programming.
I'll work on this. I've heard a couple of professors say that one thing they don't like about teaching with Java is that they have to do some hand waving about the meaning of "public static void main(String[] args)". We won't be able to get away from this line of code obviously but we may want to make it obvious that this line means the class being created is more like a "program". Even better I feel is to aid them so that before they even know what a class is they can get started programming a "program". One idea I would like to explore is the idea of creating a New Java Program button along side the New Java Class button. This of course would just be for the beginner mode. I feel the word Program would make it easier for the student to get started. The down side is that it might give them an incorrect mental model of Java.
Created attachment 158824 [details] Version 1 Constructor allows initial source code to be specified The constructor now allows the user to specify the initial source code to be included in the new class being created.
Created attachment 158827 [details] Version 1 Wizards for Programs and Classes Modified the wizards and actions so that there are now 2 actions for creating classes. - New Java Progam: Creates a Java class with a main method and some clear hints that the student should just start writing code there. This will make it easy for the beginner to get started writing code that runs. The word 'program' will make it obvious that this is a good place to start making a program if they are not familiar with what a class is yet. - New Java Class: Creates a Java class with a constructor, an instance variable with getter and setter. This is meant to be used a little later on by students once they get into the concepts of object-oriented programming. Future plans for this wizard would be to add other object oriented programming features like subclassing and implementing interfaces and abstract classes. I can't make a patch with a .gif file in it so the 'newclassprog_wiz.gif' file attached to this bug needs to be manually placed in: org.eclipse.ide4edu.javalite.ui/src/icons
Created attachment 158828 [details] Icon for 'New Java Program' action Can't make a patch that includes .gifs apparently so this needs to be placed manually in the org.eclipse.ide4edu.javalite.ui/src/icons folder.
The code in the patches simply passes a String containing the initial code into the Constructor. I did explore using Eclipse Templates (i.e. code with placeholders like ${type_name} in it) but a lot of that code for Java was internal. I thought about rebuilding a simplified version of this functionality but I thought it might be a bit too heavy duty for our modest needs at this point. Simply building the code is simpler and easier to understand.
(In reply to comment #5) > I thought about rebuilding a simplified version of this functionality but I > thought it might be a bit too heavy duty for our modest needs at this point. > Simply building the code is simpler and easier to understand. I agree - If the user is in a position where they want to have customized code templates, they would most likely be experienced enough to move on to the regular Eclipse.
First off, let me congratulate you on your use of the term "instance variables". Most people don't quite get that fields and instance variables are the same thing (and, that's what we call them in Smalltalk). Having said that, I think it's better to use the term "field" which is pretty consistently used in the Java world. I'm not comfortable with this approach. For starters, the entire idea of the constructor is to do the construction for you. Providing the constructor with very specific source code seems to spread the responsibilities for that operation around too much. Mixing up responsibilities is one of the reasons that the current JDT code is impossible for us to reuse. Right now, the wizard knows how to present information to the user and it defers all creation responsibilities to the constructor. We need to keep this clear separation. This will make reuse of the constructor for other purposes easier. I don't think it'd be too hard to add a switch or two to the constructor to let it do the heavy lifting. This might also be an opportunity to make an extension point that would allow adopters to create their own templates. The constructor can provide its clients (i.e. the wizard) with a list of available templates (that may include displayable images); the client (i.e. the wizard) can then present the options to the user in some sensible way. We could use this extensibility to permit creation of an Applet per Bug 302362. I'm concerned that we're adding complexity to the UI. I think we have an opportunity here for some UI creativity.
I like the idea of storing a collection of templates in the constructor and then allowing the wizard to select the template it would like to use. I will refactor the code to do this. Adding an extension point to the plug-in to allow other templates to be added is a good idea also for down the road. I'm not crazy about presenting a list of templates to the user in the wizard. At least not in Beginner mode anyways. I think it would be much clearer and simpler for the user to be able to create what they want (i.e. a program or a class) from the main user interface. Otherwise I feel we would be back to New Java Class and I think students will be confused initially by the word 'Class'. I experience a similar thing when I go to use PyDev after having not used it in a while. I want to create a New Python File, Program or Script but am presented with New Python Module which I always think is going to create something like a Java Package (a group of Python files). I think using as much non-Java specific language when possible in the Beginner mode of IDE4Edu is one good way to not intimidate students. I suppose we could use something like New Java File and then present a list of templates? This still seems like an extra step that is unnecessary. If we are concerned about UI clutter and complexity then when we get around to cleaning up the toolbar we should move the buttons at the top of the Package Explorer up to the main toolbar so all the buttons are in one place. As far as the clutter to the context sensitive menu, I envision that menu having the following options: - New Java Program - New Java Class - New Java Project -------------------- - Run (- Debug? maybe) -------------------- - Copy - Paste - Rename - Delete -------------------- (- Import? maybe) -------------------- (- Properties? maybe) This seems like a pretty non-intimidating list of options whose meanings are clear which I don't think is overly complex.
Created attachment 159516 [details] Version 2 Constructor allows initial source code template to be specified I've added some template related classes and have stored a list of templates in the constructor. The template of interest can be selected by name. We might want to allow it to be selected by index as well?
Created attachment 159517 [details] Version 2 Wizards for Programs and Classes This patch reworks the wizards to utilizes the new constructor that uses templates. I haven't changed the splitting of class creation into Program and Class as I am still convinced this is the best solution so far to our goal of creating a simpler, friendlier Eclipse.
This is nit-picky, but important. I find this style of programming cumbersome. public boolean selectTemplate(String templateId) { boolean foundTemplate = false; for (AbstractJavaTemplate t : templates){ if (t.getID().equals(templateId)){ currentTemplate = t; foundTemplate = true; break; } } return foundTemplate; } I understand what you're trying to do. But wouldn't this be simpler? public boolean selectTemplate(String templateId) { for (AbstractJavaTemplate template : templates){ if (template.getID().equals(templateId)) return template; } return null; } The code is tigher and it's very clear what it does. Never use a variable name like 't' when you can use a name that has some meaning. Remember that you're going to spend far more time reading this code than writing it; make it as easy as possible to read. FWIW, you're not actually using the foundTemplate value for anything. I think that we're getting closer, but I need to think a bit more before I respond more fully on your implementation.
Actually your method does something different than my method. Maybe I should rename the method to something like 'useTemplate'. This method should not return the template but should set the template that the constructor is going to use to generate a new Java class. I thought the boolean return value may be helpful for checking that the action was completed successfully. It might be extraneous. I read once that a function should only have a single return for ease of readability (!) and it stuck with me. I'll change it to the style you are suggesting. I look forward to your feedback.
Darn it. I really need to stop commenting on bugs after midnight. Groan. However, the principle still applies. How about this: public boolean selectTemplate(String templateId) { for (AbstractJavaTemplate template : templates){ if (template.getID().equals(templateId)){ currentTemplate = template; return true; } } return false; } In this case, the method returns a boolean, but isn't asking a true/false question. This is what threw me off at the late hour. A question like hasTemplate(String) or isValidTemplateId(String) or something along these lines would be expected to return a boolean value. A method name like selectTemplate is more of an imperative, "do this". No return value is implied in the name. As a practical matter, we sometimes will return a boolean value from methods, but we should do so carefully as the potential for confusion is great. If the boolean is really meant to indicate an exceptional case, then maybe an exception would be a better fit. Maybe, but not necessarily. My point is that using a boolean return value as a convenience should be carefully considered. public void hasTemplate(String templateId) { return findTemplate(templateId) != null; } public void selectTemplate(String templateId) { currentTemplate = findTemplate(templateId); } public boolean findTemplate(String templateId) { for (AbstractJavaTemplate template : templates){ if (template.getID().equals(templateId)) return template; } return null; } By factoring the actual "find" behaviour out, we can use method names to attach meaning to the code (thereby making it easier to understand), and reuse the code in different contexts.
I guess I will throw my 2 cents in too.. I think I see a bunch of potential optimizations, (bear with me, as I'm just doing a code inspection) so I will just start listing them off: 1. NewJavaClassConstructor - Is maintaining a list of templates the best way to go? Would a (static) factory design work better? Instead of using and passing around string IDs, you could just use the factory and static ints to determine a template? Then a switch statement in the factory to pick your template of choice? - Also, will we ever need to initialize templates more then once? With a (static) factory, we can branch off a little more functionality from the class constructor. Even just a private class at the bottom would be nice, and can allow for room to grow, without having to worry about intermixing code for the different functions. 2. AbstractJavaTemplate - The getters and abstract methods feel a little weird to me. For example, we have: public String getTitle() { return title; } and abstract protected String buildTemplateTitle(); Couldn't that just become abstract public String getTitle(); and let the extenders give the name there? Same thing for the description and codeSource. I would say your abstract class only needs 3 functions (in fact, you could just make it an interface tbh, but if you're sticking with the abstract class).. - abstract public String getTitle(); - abstract public String getDescription(); - abstract public String getSourceCode(); - Also, in preparation for translations, keep in mind that everytime you append "Some String", that the string is going to need to be extracted out. And it may be overkill, but large chunks of that could probably be simplified into like: - static String HEADER = "\n/**\n*/ Class description comment goes here.\n"; (Saves building it everytime, but you lose a little bit of readability). 3. The *CreationWizard extended classes - Do we need to extend NewLiteClassCreationWizard when you are only adding a new element into the constructor? I havent looked through all 5 of the above patches, but how are you determining which wizard to pick? Maybe it would be better to add a new public method into the NewLiteClassCreationWizard for picking a new template, otherwise the default is used (being just a regular empty class). And, you may be able to delegate the getClassTypeName() off to the templates themselves?
If we're going to allow some notion of arbitrary templates, then we should make it generally extensible using an extension point (or perhaps something using OSGi services). At runtime, we can use an extension point to find available templates and present them to the user. I can imagine an extension point that allows an extension to provide a title, description, some indication of relative ordering (perhaps using weights), and a string with substitution parameters. We may actually be able to do this without requiring individual classes for template. I may cobble an example of this together over the weekend for your consideration. We should probably spend some time making sure that we're not reinventing something that the Platform or JDT already provides; there is a notion of templates already, but I don't think that they're at this level (more at the level of comments and such; parts of classes).
Thanks for the feedback Cory. It's helpful to get another persons perspective on this stuff. (In reply to comment #14) > I guess I will throw my 2 cents in too.. > > I think I see a bunch of potential optimizations, (bear with me, as I'm just > doing a code inspection) so I will just start listing them off: > > 1. NewJavaClassConstructor > - Is maintaining a list of templates the best way to go? Would a (static) > factory design work better? Instead of using and passing around string IDs, you > could just use the factory and static ints to determine a template? Then a > switch statement in the factory to pick your template of choice? > - Also, will we ever need to initialize templates more then once? With a > (static) factory, we can branch off a little more functionality from the class > constructor. Even just a private class at the bottom would be nice, and can > allow for room to grow, without having to worry about intermixing code for the > different functions. I was thinking about something like this but the problem is that the templates need to have a pointer to the constructor in order to generate the source code. So the templates need to get initialized or at least have a constructor registered with them whenever they are going to be used. I suppose some kind of static factory could be used and it could have a registerConstructor(NewJavaClassConstructor constructor) method that could pass the constructor on to all of the templates it holds. This does seem like a better idea. I will look into this. In response to your comments about the String IDs, I was trying to keep with the style of Eclipse which seems to like using String IDs to select things. I think this is also more flexible as we can allow outside users to add templates to the factory/template list. > 2. AbstractJavaTemplate > - The getters and abstract methods feel a little weird to me. For example, we > have: > > public String getTitle() { > return title; > } > > and > > abstract protected String buildTemplateTitle(); > > Couldn't that just become abstract public String getTitle(); and let the > extenders give the name there? Same thing for the description and codeSource. Good point. Not sure what I was thinking there. I think I was trying to prevent it from building the description more than once but I could use some kind of test to make sure of that. > > I would say your abstract class only needs 3 functions (in fact, you could just > make it an interface tbh, but if you're sticking with the abstract class).. > > - abstract public String getTitle(); > - abstract public String getDescription(); > - abstract public String getSourceCode(); The problem is that Interfaces don't allow me to specify a constructor and I wanted to force the templates to have a class constructor field so that the templates get built properly and don't throw any NullPointerExceptions. > - Also, in preparation for translations, keep in mind that everytime you append > "Some String", that the string is going to need to be extracted out. Not every string. The code strings will be fine I think. I've never programmed in a different language. If you program in Chinese or Italian, is the main method still written 'public static void main(...' ? You are right about the comments that we will have to extract and append but I'm not sure that we can get around this. > And it may > be overkill, but large chunks of that could probably be simplified into like: > > - static String HEADER = "\n/**\n*/ Class description comment goes here.\n"; > (Saves building it everytime, but you lose a little bit of readability). I like the readability and by using the StringBuilder appending is in the average case done efficiently. > 3. The *CreationWizard extended classes > - Do we need to extend NewLiteClassCreationWizard when you are only adding a new > element into the constructor? I havent looked through all 5 of the above > patches, but how are you determining which wizard to pick? Maybe it would be > better to add a new public method into the NewLiteClassCreationWizard for > picking a new template, otherwise the default is used (being just a regular > empty class). And, you may be able to delegate the getClassTypeName() off to the > templates themselves? Good ideas. I will look into this.
(In reply to comment #15) > If we're going to allow some notion of arbitrary templates, then we should make > it generally extensible using an extension point (or perhaps something using > OSGi services). At runtime, we can use an extension point to find available > templates and present them to the user. I think this should be a separate bug or feature request. I don't see a fully extensible template system as a high priority for this project quite yet. > I can imagine an extension point that allows an extension to provide a title, > description, some indication of relative ordering (perhaps using weights), and a > string with substitution parameters. We may actually be able to do this without > requiring individual classes for template. I may cobble an example of this > together over the weekend for your consideration. I look forward to seeing this. I'm not sure exactly what you mean quite yet. > We should probably spend some time making sure that we're not reinventing > something that the Platform or JDT already provides; there is a notion of > templates already, but I don't think that they're at this level (more at the > level of comments and such; parts of classes). There is a template system for Java in the JDT but it is all internal. See CompilationUnitContextType and the various classes that subclass TemplateVariableResolver. It would be nice to use it but we would have to copy it all over to our code. I felt that since we have fairly modest needs for our templates it wasn't worth the pain in trying to rebuild all of that functionality. And you are right there is a lot of template functionality at the level of code snippet generation and comment generation and the like that we don't need since we will be using the JDT editor.
Created attachment 159735 [details] Version 3 Constructor and Templates, constructor allows initial source code template to be specified Refactored a lot of the Java templates stuff. Refactored out a collection class that manages the templates. But as per Cory's suggestion, I did not make it static. I didn't see the point of having a collection of templates hanging around all the time and I wasn't sure if synchronizing this collection might become an issue at some point. I've left out the ability to add templates to this collection through the constructor. As I thought about it more and more, the extension point for custom templates seemed like a low priority and it seemed unrelated to this bug/feature directly.
Created attachment 159738 [details] Patch to add java code templates using factory design - all inclusive Hey Miles, I had some free time and couldn't resist looking into this.. So I took all your previous patches and tried to make it a little more coherent and complete. How do you manage with 5 different patching 4 different things, but they are all required to function, and some even had overlap =/ - very confusing for me to try and get up to speed. May I recommend just keep posting 1 complete patch if you make changes? However, try this proposed patch - it should include all changes, templates, wizards, and actions - its all inclusive. I refactored the templates into more of a Factory design (see the new JavaTemplateFactory class), and made the AbstractJavaTemplate into just a regular interface (now called JavaTemplate). I then tried to sort through how they were being used through the wizard / action side. I removed your additional object / program classes. The regular class action / wizard now take a JavaTemplate, and will attempt to pull details directly from the templates themselves to populate the wizard, and also for the final output source code upon creation. In LitePackageExplorer, you can see how I modified the actions to create java classes using specific templates. And as a last note, this patch also includes the patch posted for Bug 301326 (fixing the potential NPE), and my DeleteHandler updates (see Bug 303341 for more info), as they share the same classes and I don't feel like juggling 300 patches.
(In reply to comment #19) > And as a last note, this patch also includes the patch posted for Bug 301326 > (fixing the potential NPE), and my DeleteHandler updates (see Bug 303341 for > more info), as they share the same classes and I don't feel like juggling 300 > patches. Sorry, small correction: This patch only includes a partial patch of the other DeleteHandler. (patch will still compile if applied to a clean workspace, but the delete button wont work). Also, it just occured to me... these patches aren't really addressing the actual bug right? The bug is that "we should have a static main method toggle" inside the wizard. But I suppose its a step in the right direction.
(In reply to comment #19) > How do you manage with 5 different patching 4 different things, > but they are all required to function, and some even had overlap =/ - very > confusing for me to try and get up to speed. May I recommend just keep posting > 1 complete patch if you make changes? The 5 patches aren't all required. Wayne mentioned that we should submit separate patches for things that are in different plug-ins. The constructor patches all modify just the javalite.constructors plug-in and the wizard patches modify just the javalite.ui plug-in. To get Version 2 working you should just need the two Version 2 patches and the icon image attachment. The Version 3 patch only affects the javalite.constructor plugin. It works with the Version 2 wizard patch. I suppose for clarity I should have attached a copy of the Version 2 wizard patch as the Version 3 wizard patch. I'll do that.
Created attachment 159744 [details] Version 3 Wizards for Programs and Classes The same as Version 2. Just attached so it's clear what patches to apply to get Version 3 working.
(In reply to comment #20) > Also, it just occured to me... these patches aren't really addressing the > actual bug right? The bug is that "we should have a static main method toggle" > inside the wizard. But I suppose its a step in the right direction. No these patches are addressing that bug... just at a higher level. The real problem that this bug is describing is how to give the option to a student to create a class, when they don't know what a class is, with a main method, when they don't know what a main method is (let alone what "public static void main(String[] args)" means). I'm proposing an abstraction that removes this functionality from the wizard and puts it in the primary UI. That abstraction is the idea of a Java Program and of a Java Class. The IDE functionality requirements of a beginning student are very, very small so my thinking is that it is better to have as much of this limited functionality out in the open and not hidden away in wizards. Also, it allows us to not have to try to get the students to do things they don't understand. They don't know what a class is so it is not obvious that they should press New Java Class. They don't know what a main method is so it is not obvious that they should press a toggle to include a main method. The do know what a Program is and they do want to start programming a program so I think it is obvious that they would know to press New Java Program to get started. Do you think this is a good solution?
Created attachment 160444 [details] Version 4 Constructor and Templates, constructor allows initial source code template to be specified Moved more data into the templates so that the templates drive not only the source code being generated but allow for hooks into descriptions and titles related to the template content that can be used by the NewLiteClassCreationWizard to describe the content to be generated. Changed a bit of language in the templates.
Created attachment 160445 [details] Version 4 Wizards for Programs and Classes Allow IJavaTemplates to control what kind of class the wizard creates and to specify some of the language used in the labels in the Wizard. Different than Version 3. Now the type of wizard to be created is just controlled by the OpenNewLiteClass*WizardAction and we don't have separate wizards for each type of class being created. I think I like allowing IJavaTemplate files to drive some of the functionality and language of the wizards. Thanks to Cory for the idea.
Created attachment 160839 [details] Partial implementation using extension-point Like I mentioned in comment 14, I think we can do this with extension points and a little templating. This attachment provides a starting point for what I'm thinking of with a couple of hints on what needs to be done next. The key takeaway from this is that this implementation is easily extended by adding a new plug-in that provides an extention to the extension-point I've defined.
I've started redoing what I've done so far using the extension point rather than IJavaTemplates. Wayne, how far down the rabbit hole do you recommend I go with this template thing? Just curious how you would approach this. Should I just get it working, managing all the replacing myself using the TemplateTranslator, TemplateBuffer and TemplateVariables? Or would you go the whole hog and figure out TextEdits and stuff?
Nevermind. Ignore that last post.
I can't ignore... I dug into the Template stuff and decided that maybe some regular expressions find and replace might perform adequately...
Actually you had a note about the TemplateTranslator in the code. I got that working and then I found this http://help.eclipse.org/help32/index.jsp?topic=/org.eclipse.platform.doc.isv/reference/api/org/eclipse/text/edits/TextEdit.html and this looks like it will work well given we'll have a list of TemplateVariables with offsets and lengths from the TemplateTranslator.
Created attachment 161257 [details] Constructor and Templates using extension point Builds upon the extension point that Wayne built to allow different templates to be used in the constructor. Allows users of the constructor to select the template associated with the constructor. And the code generated with the constructor will have the correct class name and package name filled in where required.
Created attachment 161258 [details] Version 5 Wizards for Programs and Classes Similar to Version 3 only uses the new methods of selecting a template for the NewJavaClassConstructor.
I've applied the constructor patch and have marked it as iplog+. I like where this is going. I'm not particularly fond of the UI change, however. I think we can do better. Your creativity is appreciated.
