You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by "Michael Wyraz (JIRA)" <ji...@apache.org> on 2010/05/13 10:12:43 UTC

[jira] Created: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
----------------------------------------------------------------------------

                 Key: TAP5-1148
                 URL: https://issues.apache.org/jira/browse/TAP5-1148
             Project: Tapestry 5
          Issue Type: Bug
    Affects Versions: 5.0.19
            Reporter: Michael Wyraz
            Priority: Trivial


The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.

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


[jira] Commented: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

Posted by "Hugo Palma (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/TAP5-1148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12927524#action_12927524 ] 

Hugo Palma commented on TAP5-1148:
----------------------------------

This is such a simple change, can anyone please pick this up ?

> org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
> ----------------------------------------------------------------------------
>
>                 Key: TAP5-1148
>                 URL: https://issues.apache.org/jira/browse/TAP5-1148
>             Project: Tapestry 5
>          Issue Type: Bug
>    Affects Versions: 5.0.19
>            Reporter: Michael Wyraz
>            Priority: Trivial
>
> The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.
> These methods should also be protected, not private.

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


[jira] Commented: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

Posted by "Robert Zeigler (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/TAP5-1148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12927584#action_12927584 ] 

Robert Zeigler commented on TAP5-1148:
--------------------------------------

Looking through the code, the only generally useful utility method is "inError" (and maybe addErrorClassToCurrentElement).   But both of these are one-liners.

The methods only need to be protected if the class is extensible.

I'm not keen on removing the "final" restriction on DefaultValidationDecorator.  It's internal, and shouldn't be referenced directly, anyway.  If you want the behavior of the default decorator + some additional behavior, there are better ways to get there (eg: push a validation decorator onto the environment that wraps the default validation decorator) that cushion you against potentially backwards incompatible changes.

I considered moving "inError" up to BaseValidationDecorator.  But that would introduce dependencies on BaseValidationDecorator (Environment, MarkupWriter) that would break any code extending from BaseValidationDecorator.

One option would be to introduce another validator into the hierarchy, between BaseValidationDecorator and DefaultValidationDecorator.  This decorator would not override any of the methods in the ValidationDecorator interface, but would contain the methods requested, as protected methods.  That seems like more clutter.

Really, are the one-liners so onerous?  I'm willing to change my mind, but at the moment, I'm inclined to resolve this as invalid.

> org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
> ----------------------------------------------------------------------------
>
>                 Key: TAP5-1148
>                 URL: https://issues.apache.org/jira/browse/TAP5-1148
>             Project: Tapestry 5
>          Issue Type: Bug
>    Affects Versions: 5.0.19
>            Reporter: Michael Wyraz
>            Priority: Trivial
>
> The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.
> These methods should also be protected, not private.

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


[jira] Updated: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

Posted by "Michael Wyraz (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/TAP5-1148?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael Wyraz updated TAP5-1148:
--------------------------------

    Description: 
The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.

These methods should also be protected, not private.

  was:The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.


> org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
> ----------------------------------------------------------------------------
>
>                 Key: TAP5-1148
>                 URL: https://issues.apache.org/jira/browse/TAP5-1148
>             Project: Tapestry 5
>          Issue Type: Bug
>    Affects Versions: 5.0.19
>            Reporter: Michael Wyraz
>            Priority: Trivial
>
> The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.
> These methods should also be protected, not private.

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


[jira] Updated: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

Posted by "Michael Wyraz (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/TAP5-1148?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael Wyraz updated TAP5-1148:
--------------------------------

    Description: 
The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.

These methods should also be protected, not private.

  was:The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.


> org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
> ----------------------------------------------------------------------------
>
>                 Key: TAP5-1148
>                 URL: https://issues.apache.org/jira/browse/TAP5-1148
>             Project: Tapestry 5
>          Issue Type: Bug
>    Affects Versions: 5.0.19
>            Reporter: Michael Wyraz
>            Priority: Trivial
>
> The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.
> These methods should also be protected, not private.

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


[jira] Commented: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

Posted by "Hugo Palma (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/TAP5-1148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12927655#action_12927655 ] 

Hugo Palma commented on TAP5-1148:
----------------------------------

So wrapping the DefaultValidationDecorator is the recommended way to add functionality instead of extending it ?

If so, i can live with that.

> org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
> ----------------------------------------------------------------------------
>
>                 Key: TAP5-1148
>                 URL: https://issues.apache.org/jira/browse/TAP5-1148
>             Project: Tapestry 5
>          Issue Type: Bug
>    Affects Versions: 5.0.19
>            Reporter: Michael Wyraz
>            Priority: Trivial
>
> The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.
> These methods should also be protected, not private.

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


[jira] Commented: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

Posted by "Hugo Palma (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/TAP5-1148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12927655#action_12927655 ] 

Hugo Palma commented on TAP5-1148:
----------------------------------

So wrapping the DefaultValidationDecorator is the recommended way to add functionality instead of extending it ?

If so, i can live with that.

> org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
> ----------------------------------------------------------------------------
>
>                 Key: TAP5-1148
>                 URL: https://issues.apache.org/jira/browse/TAP5-1148
>             Project: Tapestry 5
>          Issue Type: Bug
>    Affects Versions: 5.0.19
>            Reporter: Michael Wyraz
>            Priority: Trivial
>
> The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.
> These methods should also be protected, not private.

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


[jira] Commented: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

Posted by "Robert Zeigler (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/TAP5-1148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12927584#action_12927584 ] 

Robert Zeigler commented on TAP5-1148:
--------------------------------------

Looking through the code, the only generally useful utility method is "inError" (and maybe addErrorClassToCurrentElement).   But both of these are one-liners.

The methods only need to be protected if the class is extensible.

I'm not keen on removing the "final" restriction on DefaultValidationDecorator.  It's internal, and shouldn't be referenced directly, anyway.  If you want the behavior of the default decorator + some additional behavior, there are better ways to get there (eg: push a validation decorator onto the environment that wraps the default validation decorator) that cushion you against potentially backwards incompatible changes.

I considered moving "inError" up to BaseValidationDecorator.  But that would introduce dependencies on BaseValidationDecorator (Environment, MarkupWriter) that would break any code extending from BaseValidationDecorator.

One option would be to introduce another validator into the hierarchy, between BaseValidationDecorator and DefaultValidationDecorator.  This decorator would not override any of the methods in the ValidationDecorator interface, but would contain the methods requested, as protected methods.  That seems like more clutter.

Really, are the one-liners so onerous?  I'm willing to change my mind, but at the moment, I'm inclined to resolve this as invalid.

> org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
> ----------------------------------------------------------------------------
>
>                 Key: TAP5-1148
>                 URL: https://issues.apache.org/jira/browse/TAP5-1148
>             Project: Tapestry 5
>          Issue Type: Bug
>    Affects Versions: 5.0.19
>            Reporter: Michael Wyraz
>            Priority: Trivial
>
> The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.
> These methods should also be protected, not private.

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


[jira] Commented: (TAP5-1148) org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final

Posted by "Hugo Palma (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/TAP5-1148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12927524#action_12927524 ] 

Hugo Palma commented on TAP5-1148:
----------------------------------

This is such a simple change, can anyone please pick this up ?

> org.apache.tapestry5.internal.DefaultValidationDecorator sounld not be final
> ----------------------------------------------------------------------------
>
>                 Key: TAP5-1148
>                 URL: https://issues.apache.org/jira/browse/TAP5-1148
>             Project: Tapestry 5
>          Issue Type: Bug
>    Affects Versions: 5.0.19
>            Reporter: Michael Wyraz
>            Priority: Trivial
>
> The class contains usefull code which has to be copied for an own component (bad practice). So either this code (e.g. inError(Field) should go to BaseValidationDecorator or the clas should allowed to be a superclass.
> These methods should also be protected, not private.

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