You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "Mathias Bogaert (JIRA)" <ji...@apache.org> on 2008/11/27 11:30:37 UTC

[jira] Created: (WW-2896) Various generics and StringBuilder fixes

Various generics and StringBuilder fixes
----------------------------------------

                 Key: WW-2896
                 URL: https://issues.apache.org/struts/browse/WW-2896
             Project: Struts 2
          Issue Type: Improvement
          Components: Core Actions
    Affects Versions: 2.1.2
            Reporter: Mathias Bogaert
             Fix For: 2.1.3
         Attachments: variousgenericsbuilder.patch



-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2896) Various generics and StringBuilder fixes

Posted by "Musachy Barroso (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Musachy Barroso updated WW-2896:
--------------------------------


moving to future, this is a long task and it is not something we need to do right away, volunteers welcome :)

> Various generics and StringBuilder fixes
> ----------------------------------------
>
>                 Key: WW-2896
>                 URL: https://issues.apache.org/struts/browse/WW-2896
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.2
>            Reporter: Mathias Bogaert
>             Fix For: Future
>
>         Attachments: variousgenericsbuilder.patch, variousgenericsbuilder.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2896) Various generics and StringBuilder fixes

Posted by "Musachy Barroso (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Musachy Barroso updated WW-2896:
--------------------------------

    Fix Version/s:     (was: 2.1.3)
                   2.1.4

moving to 2.1.4. The s/StringBuffer/StringBuilder changes are fine, but using generics all over the place is something we can do on 2.1.4

> Various generics and StringBuilder fixes
> ----------------------------------------
>
>                 Key: WW-2896
>                 URL: https://issues.apache.org/struts/browse/WW-2896
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.2
>            Reporter: Mathias Bogaert
>             Fix For: 2.1.4
>
>         Attachments: variousgenericsbuilder.patch, variousgenericsbuilder.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2896) Various generics and StringBuilder fixes

Posted by "Mathias Bogaert (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mathias Bogaert updated WW-2896:
--------------------------------

    Attachment: variousgenericsbuilder.patch

> Various generics and StringBuilder fixes
> ----------------------------------------
>
>                 Key: WW-2896
>                 URL: https://issues.apache.org/struts/browse/WW-2896
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.2
>            Reporter: Mathias Bogaert
>             Fix For: 2.1.3
>
>         Attachments: variousgenericsbuilder.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2896) Various generics and StringBuilder fixes

Posted by "Wes Wannemacher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Wes Wannemacher updated WW-2896:
--------------------------------

    Fix Version/s:     (was: 2.1.4)
                   2.1.5

> Various generics and StringBuilder fixes
> ----------------------------------------
>
>                 Key: WW-2896
>                 URL: https://issues.apache.org/struts/browse/WW-2896
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.2
>            Reporter: Mathias Bogaert
>             Fix For: 2.1.5
>
>         Attachments: variousgenericsbuilder.patch, variousgenericsbuilder.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2896) Various generics and StringBuilder fixes

Posted by "Musachy Barroso (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Musachy Barroso updated WW-2896:
--------------------------------

    Fix Version/s:     (was: 2.1.7)
                   Future

> Various generics and StringBuilder fixes
> ----------------------------------------
>
>                 Key: WW-2896
>                 URL: https://issues.apache.org/struts/browse/WW-2896
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.2
>            Reporter: Mathias Bogaert
>             Fix For: Future
>
>         Attachments: variousgenericsbuilder.patch, variousgenericsbuilder.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2896) Various generics and StringBuilder fixes

Posted by "Wes Wannemacher (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45057#action_45057 ] 

Wes Wannemacher commented on WW-2896:
-------------------------------------

Mathias, I appreciate your effort, but these patches are rather large... I am going to start going through them and post a few questions here to illicit feedback from other committers. 

1. 
First off, in the first patch, posted 11/27, this change to Component.java- 
-            Map params = getParameters();
-
             if (value == null) {
-                params.remove(key);
+                getParameters().remove(key);
             } else {
-                params.put(key, value);
+                getParameters().put(key, value);
             }

I'm not a fan of calling a method more than once when a scoped variable will save the method call. The difference in memory/cpu usage is negligible, but if this section is expanded, having the variable available seems more appropriate. 

2.
In TagModel.java, the following change - 
-    public Writer getWriter(Writer writer, Map params)
-        throws TemplateModelException, IOException {
+    public Writer getWriter(Writer writer, Map params) throws TemplateModelException, IOException {

is not gonna go in because we try to keep lines less than 78 characters[*], there are a few others in that same file.
*citation needed

3.
In core/src/site/resources/tags/ajax/autocompleter.html, it seems like building core generates the following change - 

-					<td align="left" valign="top">String</td>
+					<td align="left" valign="top">Integer</td>

I've been seeing that show up today, I haven't committed it, so if anyone knows what's up, I'd like to know.

4. 
The change to PortletActionRedirectResult shows how using generics cleans up code, but the spacing looks like it got skewed. (Also, it appears that not everyone was keeping with the 78 char width in the trunk copy)

5. 
In MessageStoreInterceptor, it seems like there are a lot of whitespace changes, which I like to avoid committing (since it generates email spam)

6. 
In ContainUtil.java, I am not convinced that adding an 'else' is the right thing to do. Following the logic of the code, it seems like there is a possibility that an object can be a Map that does not contain obj2, but obj1 is iterable and does contain obj2. After reading though, it would seem that the section that checks if obj1 is a Map could be shortened by performing both tests ( if (obj1 instanceof Map && ((Map) obj1).containsKey(obj2)) return true).

All in all, this was obviously quite a bit of work and I am sure that I am not the only one that appreciates you doing it. I may bump this to 2.1.4 because I am interested in getting a 2.1.3 release as quick as possible.

> Various generics and StringBuilder fixes
> ----------------------------------------
>
>                 Key: WW-2896
>                 URL: https://issues.apache.org/struts/browse/WW-2896
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.2
>            Reporter: Mathias Bogaert
>             Fix For: 2.1.3
>
>         Attachments: variousgenericsbuilder.patch, variousgenericsbuilder.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2896) Various generics and StringBuilder fixes

Posted by "Dave Newton (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45063#action_45063 ] 

Dave Newton commented on WW-2896:
---------------------------------

4.
The change to PortletActionRedirectResult shows how using generics cleans up code, but the spacing looks like it got skewed. (Also, it appears that not everyone was keeping with the 78 char width in the trunk copy)

Oh, were the StringBuilder patches only partially applied? I fixed PortletActionRedirectResult yesterday and discovered IDEA is removing blank chars at the end of lines so the commit generated a bunch of noise. I know it's noise, but... meh.

I'm not a big fan of 78-chars-per-line these days, either :( not practical in real Java code. We can move this to dev list to continue.

> Various generics and StringBuilder fixes
> ----------------------------------------
>
>                 Key: WW-2896
>                 URL: https://issues.apache.org/struts/browse/WW-2896
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.2
>            Reporter: Mathias Bogaert
>             Fix For: 2.1.3
>
>         Attachments: variousgenericsbuilder.patch, variousgenericsbuilder.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2896) Various generics and StringBuilder fixes

Posted by "Mathias Bogaert (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mathias Bogaert updated WW-2896:
--------------------------------

    Attachment: variousgenericsbuilder.patch

More fixes.

> Various generics and StringBuilder fixes
> ----------------------------------------
>
>                 Key: WW-2896
>                 URL: https://issues.apache.org/struts/browse/WW-2896
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.2
>            Reporter: Mathias Bogaert
>             Fix For: 2.1.3
>
>         Attachments: variousgenericsbuilder.patch, variousgenericsbuilder.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.