[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Updates: Status: Fixed Comment #16 on issue 99 by mr0...@mro.name: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 again and more profound: r602. Please Verify and vote. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #15 on issue 99 by mr0...@mro.name: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 undone in r598. I'll be back. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Updates: Status: Started Comment #14 on issue 99 by mr0...@mro.name: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 needs more work, see discussion at r563. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Updates: Status: Fixed Comment #13 on issue 99 by mr0...@mro.name: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 please verify and vote r563 -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #12 on issue 99 by mr0...@mro.name: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 I'll just remove the toString implementation entirely, so they fall back to Object. Consensus not yet, Allain has his pain with removing a functioning method that's a pleasure to the stackoverflowers but I think we two are pretty much aligned. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #11 on issue 99 by heuermh: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 Do what exactly? Have we reached consensus on this yet? -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Updates: Owner: mr0...@mro.name Comment #10 on issue 99 by mr0...@mro.name: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 I'll do the job on Saturday, expect some commit activity between 12:00-14:00 UTC. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #9 on issue 99 by allain.lalonde: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 http://stackoverflow.com/questions/1161228/tostring-in-java 'nuf said, but of course majority rules, just doesn't seem to be the majority of java users. :) Oh well. I'll let one of you two make these changes since I think they're terrible. Anyway, there are bigger fish to fry. Onto bigger and brighter bugs. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #8 on issue 99 by heuermh: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 I agree with Allain in that paramString methods smell bad. I also see them of little-to-no utility. If someone wants to debug, use a debugger. My full suggestion is to 1. Remove any implementation of toString(), defaulting to Object.toString() 2. Mark all paramString() methods as @deprecated 3. Refactor all paramString() methods to call toString() 4. Release version 1.3 with them @deprecated and delegating to toString() 5. Remove all paramString() methods for the next release PParamStringBuilder can be added to extras now or later independently. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #7 on issue 99 by mr0...@mro.name: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 Hi Michael, if we want to keep the dubugging output of toString, I'd second Allain in adding a new class to the core. Expanding the interface gives me quite a headache, but for the sake of DRY-ness of the toStrings, I follow Allains argument. An external dependency is a no-go. Maybe a reflected toString is an option? What's your concerns about adding the class (which could be deprecated from the beginning)? P.S.: Ceterum censeo toString esse delendam - see comment 5 #c5. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #6 on issue 99 by allain.lalonde: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 Very strange, majority rules I guess. PParamStringBuilder is basically the same thing as the ToStringBuilder class, and the change is binary compatible to the current code. Oh, well. Should this be marked as "Will Not Fix" then? -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #5 on issue 99 by mr0...@mro.name: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 I agree to Michael and suggest, as a quick solution, do as the documentation explicitly allows: return ""; -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #4 on issue 99 by heuermh: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 Sorry, I meant point to as in documentation, not as in adding a dependency. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #3 on issue 99 by allain.lalonde: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 Sure. Of course we'd be adding an external dependency for something rather trivial. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Comment #2 on issue 99 by heuermh: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 My opinion would be to @deprecate all the paramString methods and remove them in the next released version. I would rather not add the new class PParamStringBuilder to core. It might fit in extras/.../piccolox/util, or we might point to external code like http://commons.apache.org/lang/api/org/apache/commons/lang/builder/ToStringBuilder.html http://commons.apache.org/lang/api/org/apache/commons/lang/builder/ReflectionToStringBuilder.html for those who wish to implement a paramString()-like toString() method in their own subclasses of PNode. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Updates: Status: Accepted Comment #1 on issue 99 by allain.lalonde: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 If no one has objections, I'll be folding this in soon. It makes extending toString in a consistent way less error prone. I realize I'm talking about toString() being hard, and it's usually dead simple, but because of all the null checks the cyclomatic complexity in most of the toString methods in the framework is quite high. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
[piccolo2d-dev] Issue 99 in piccolo2d: PNode.toString refactoring
Status: Started Owner: allain.lalonde Labels: Type-Enhancement Priority-Medium Effort-Low Toolkit-Piccolo2D.Java Component-Core Milestone-1.3 Maintainability New issue 99 by allain.lalonde: PNode.toString refactoring http://code.google.com/p/piccolo2d/issues/detail?id=99 With all of its string concatenation and null value checking, PNode.paramString is a bug magnet. I'd suggest that anything we can do to make it easier for users subclassing PNode, the better. Attached you will find a patch that changes PNode's toString method to use a PParamStringBuilder object instead. It makes writing nicer toStrings trivial, and drops cyclomatic complexity of the whole code base to boot. To retain binary compatability, I've made it use paramString() in case users had already implemented classes making use of it, but it has been marked as deprecated. Attachments: PNode.toString-refactoring.diff 22.3 KB -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ Piccolo2D Developers Group: http://groups.google.com/group/piccolo2d-dev?hl=en -~--~~~~--~~--~--~---
