Sunday, June 28, 2009

All contributions are Equal, some are more equal than others

This evening I read Cay Hortmann's article on upgrading to Galileo. (For those of you who do not know Cay, he writes the fantastic Core Java books. I learned Java from the 1.1 version, as well as both the 1.4 and 1.5 versions. He writes great stuff.)

In the article he makes this comment:
There is now an option to generate toString automatically. This is something I've wanted for a long time. Unfortunately, it is not very good. Core Java gives these simple rules for toString:
  • Use getClass().getName() to print the class name. Then your toString can be inherited.
  • When you redefine toString in a subclass, first call super.toString(), then add the subclass data.
The Eclipse formatter follows neither of these eminently sensible rules. Maybe in 3.6.
I've not yet seen the toString generator, but having written more than enough toString implementations, I've worried that such a code generator may be less than ideal. Similarly, I've seen the equals and hashCode generators at work, and I also tend to not want to use them.

Given that I work for a company that has so much Java code that "If code was ice cream, it would be a lot of ice cream," [Ref Java Posse 240, 9:15] in addition to having very helpful helper methods for hashCode and equals, I'd love more finely-tuned generators for such critical methods.

But I feel a bit constrained by one of the basic rules guiding Eclipse contributions, as defined by the book Eclipse: Principles, Patterns and Plug-ins:
As the Contribution Rule reminds us, “Everything is a contribution.” And with many contributions, the possibilities are endless.
OK, so let's say I write my own generators for toString, hashCode and equals. Then there are two toString generators in the IDE, one possibly better than the other. Why should the end user be forced to deal with two confusing UI contributions?

Sure, there's also the recent object of my affections, patch fragments, but the patch fragment is a back-door lover.

I want to remove the default contributions altogether, and replace them with something better suited to my environment. But that violates the Contribution Rule. Am I stuck with something less than ideal until Eclipse 3.6, or worse, forever?

What's the solution here?

PS: Has someone picked up on Cay's concerns and logged a bug?

12 comments:

http://www.lemmster.de said...

Why not file a bug first and find out if it really gets postponed until 3.6? My be a chance that it goes into 3.5.1.
In the mean time why not configure a custom toString builder that follows Cay's rules instead of writing something completely new?

In general I agree with you though, it's annoying that one has to wait ~a year for simple things to get fixed. It's the downside of a big bang release train.

Radek Urbas said...

+1 for submitting a ticket

konberg said...

Lemmster, I'm not writing a toString implementation mostly because it's not my complaint.

My real question is about how to appropriately violate the Contribution Rule in a case where you think the contribution is doing more harm than good, and given how cluttered menus are, there needs to be a higher bar for including content, particularly when they're included in such critical and ubiquitous plug-ins.

David Plass said...

Is that podcast the source of the "Kevin, tell me a story" story?

konberg said...

David, it's the source of that particular part of the "Tell me a story" story.

Eric Rizzo said...

I think the answer lies in extension points. I don't know if the current implementation of the toString generator uses an extension point to make it configurable, but it should. If it does not I would suggest providing a patch that makes it so, and then include an impl of that extension point that implements it as-is and another that implements it the way you would prefer. Then you can experiment with different UIs to present the choice: maybe a sub-menu, maybe a Preference setting, etc.

Now that I think about it, I'm surprised this is not already a code template as opposed to a completely separate function; most every part of code that is generated by JDT uses a user-editable template. Maybe that is what the bug report should be, to make the generated toString an editable template.
Might just be simpler to implement as an extension point, however; I'm not sure how complex the template code might get.

Mateusz Matela said...

As the main contributor of toString generator I'm glad to see people discussing it :)
First I'd like to defend my work a bit, although I realize complains about it were not the main point of this post. So: using getClass().getName() or including super.toString() is just a matter of configuration (it can be defined in 'format template').

Now to the main point. I think the problem mentioned is valid only if you assume that a new feature can do more harm than good. How could that happen? New features are never added by one person, they're carefully discussed by several experienced developers. toString generator was supposed to be an example of such a harmful addition, but I hope I showed that is not the case (we can discuss more if you don't agree ;>). Any other examples?

Of course no feature in a program can fit all user needs perfectly, but I believe it's always possible to satisfy 95%-99% of users. The rest of the users have so specific issues that they won't bother to create separate plug-ins to address them. They'll just find their own workarounds.

@Eric Rizzo
Yeah, it would be ideal to have each feature as a separate extension, but sometimes it's hard to accomplish and toString() generator is one of those cases. It uses a lot of internal JDT stuff... I've prepared plug-in version of the generator, but I had to copy big parts of internal classes and still the integration with IDE was worse than it is now.

Code templates seem a good idea, but it's also a very complicated issue. Simple templates that are already implemented in Eclipse won't handle such complex stuff as toString method. Something more resembling xslt would be necessary. I tried to describe such a mechanism here (in the bottom half of the post).

konberg said...

@Mateusz,

Thanks for getting involved in this conversation.

By all means, defend your work! I'd like to point out though that I am not the one unhappy with the implementation, it's Cay Horstmann. I haven't even tried your work. I recommend you make the comment to him. He may have really good feedback for you.

I do assume that a new feature can do more harm than good. There is only so much screen real-estate. When context-sensitive menu items fail to have a reasonable context, beginner users are consistently overwhelmed by the large selection of options and I have to stare at a large set of choices just to find something I use on a regular basis, I worry that Eclipse's default feature set can become too big.

You assume that the toString implementation can't be wrong because several experienced engineers discussed it, yet look at Cay's blog: he points out two conceivably important problems. So maybe that isn't enough.

I repeat the end of my first reply to lemmster: there needs to be a higher bar for including content, particularly when they're included in such critical and ubiquitous plug-ins.

Unknown said...

Good discussion here! Mateusz explained to me how one can change the template--it is pretty flexible. So the complaint really is with the default. A better default would be to always call super.toString(), followed by the bracketed list of field name/values. (1) If the class inherits from Object (or another class that properly gets the class name with getClass().getName()), then the correct class name is reported. (2) If the superclass produces a useful toString(), it is not swept under the rug. There is the minor issue that Object.toString() includes the hash code, so that you'd get something like com.horstmann.Employee@1729[name=Harry,salary=89000], but so what...

Can a new feature do more harm than good? Ya betcha. Some students will happily click their way through to a bad toString and then argue with the instructor--after all, the instructor is merely a fallible person, but Eclipse is this perfect tool that never makes a mistake.

I'd be happy to report a bug, but I'd be even happier if Mateusz could be convinced to change the default.

Eric Rizzo said...

"I'd be happy to report a bug, but I'd be even happier if Mateusz could be convinced to change the default."

Entering a bug is the way to convince the committers (Mateusz included) to change something. Even better if you provide a patch that makes the change you are suggesting.

Mateusz Matela said...

OK, I've submitted a bug for it. I'm not a commiter though, so I'm not the one that has to be convinced :)

See https://bugs.eclipse.org/bugs/show_bug.cgi?id=282291 if you want to provide more feedback or check how things develop.

Anonymous said...
This comment has been removed by a blog administrator.