You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Igor Vaynberg <ig...@gmail.com> on 2012/11/25 21:30:23 UTC

Re: [2/2] git commit: Make 'groups' member final and rename other field members to not have '_' as suffix to make them consistent with rest Wicket classes.

can you please rename the fields back.

the underscore at the end of the field means those fields should not
be accessed directly

for example property_ should not be accessed directly - only via
getProperty() getter because it is lazily initialized. even though it
is private people modifying the code may not be aware of or not
remember not to access it directly.

-igor

On Sun, Nov 25, 2012 at 7:49 AM,  <mg...@apache.org> wrote:
> Make 'groups' member final and rename other field members to not have '_' as suffix to make them consistent with rest Wicket classes.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/9e970024
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/9e970024
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/9e970024
>
> Branch: refs/heads/master
> Commit: 9e9700241eadf93bed2d2a4d61e7cadb998005d5
> Parents: ddf53c9
> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
> Authored: Sun Nov 25 16:43:05 2012 +0100
> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
> Committed: Sun Nov 25 16:43:05 2012 +0100
>
> ----------------------------------------------------------------------
>  .../wicket/bean/validation/PropertyValidator.java  |   28 +++++++-------
>  1 files changed, 14 insertions(+), 14 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/9e970024/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> ----------------------------------------------------------------------
> diff --git a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> index ea843c1..e625f38 100644
> --- a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> +++ b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> @@ -52,11 +52,11 @@ import org.apache.wicket.validation.IValidator;
>   */
>  public class PropertyValidator<T> extends Behavior implements IValidator<T>
>  {
> -       private static Class<?>[] EMPTY = new Class<?>[0];
> +       private static final Class<?>[] EMPTY = new Class<?>[0];
>
>         private FormComponent<T> component;
> -       private Property property_;
> -       private IModel<Class<?>[]> groups_;
> +       private Property property;
> +       private final IModel<Class<?>[]> groups;
>
>         public PropertyValidator(Class<?>... groups)
>         {
> @@ -75,16 +75,16 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
>
>         public PropertyValidator(Property property, IModel<Class<?>[]> groups)
>         {
> -               this.property_ = property;
> -               this.groups_ = groups;
> +               this.property = property;
> +               this.groups = groups;
>         }
>
>         private Property getProperty()
>         {
> -               if (property_ == null)
> +               if (property == null)
>                 {
> -                       property_ = BeanValidationConfiguration.get().resolveProperty(component);
> -                       if (property_ == null)
> +                       property = BeanValidationConfiguration.get().resolveProperty(component);
> +                       if (property == null)
>                         {
>                                 throw new IllegalStateException(
>                                         "Could not resolve Property from component: " +
> @@ -94,16 +94,16 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
>                                                 " to resolve the Property automatically");
>                         }
>                 }
> -               return property_;
> +               return property;
>         }
>
>         private Class<?>[] getGroups()
>         {
> -               if (groups_ == null)
> +               if (groups == null)
>                 {
>                         return EMPTY;
>                 }
> -               return groups_.getObject();
> +               return groups.getObject();
>         }
>
>         @SuppressWarnings("unchecked")
> @@ -123,7 +123,7 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
>                                 " can only be added to FormComponents");
>                 }
>
> -               // TODO add a validation key that appends the type so we can have differnet messages for
> +               // TODO add a validation key that appends the type so we can have different messages for
>                 // @Size on String vs Collection - done but need to add a key for each superclass/interface
>
>                 this.component = (FormComponent<T>)component;
> @@ -136,9 +136,9 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
>         public void detach(Component component)
>         {
>                 super.detach(component);
> -               if (groups_ != null)
> +               if (groups != null)
>                 {
> -                       groups_.detach();
> +                       groups.detach();
>                 }
>         }
>
>

Re: [2/2] git commit: Make 'groups' member final and rename other field members to not have '_' as suffix to make them consistent with rest Wicket classes.

Posted by Martin Grigorov <mg...@apache.org>.
Reverted


On Sun, Nov 25, 2012 at 9:50 PM, Martin Grigorov <mg...@apache.org>wrote:

> I think comments/javadoc in the code would be better.
> The trailing underscore didn't tell me that this is the reason.
>
> If we are going to use them from now on then let's make it public in dev@
> .
>
>
> On Sun, Nov 25, 2012 at 9:30 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> can you please rename the fields back.
>>
>> the underscore at the end of the field means those fields should not
>> be accessed directly
>>
>> for example property_ should not be accessed directly - only via
>> getProperty() getter because it is lazily initialized. even though it
>> is private people modifying the code may not be aware of or not
>> remember not to access it directly.
>>
>> -igor
>>
>> On Sun, Nov 25, 2012 at 7:49 AM,  <mg...@apache.org> wrote:
>> > Make 'groups' member final and rename other field members to not have
>> '_' as suffix to make them consistent with rest Wicket classes.
>> >
>> >
>> > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/9e970024
>> > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/9e970024
>> > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/9e970024
>> >
>> > Branch: refs/heads/master
>> > Commit: 9e9700241eadf93bed2d2a4d61e7cadb998005d5
>> > Parents: ddf53c9
>> > Author: Martin Tzvetanov Grigorov <mg...@apache.org>
>> > Authored: Sun Nov 25 16:43:05 2012 +0100
>> > Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
>> > Committed: Sun Nov 25 16:43:05 2012 +0100
>> >
>> > ----------------------------------------------------------------------
>> >  .../wicket/bean/validation/PropertyValidator.java  |   28
>> +++++++-------
>> >  1 files changed, 14 insertions(+), 14 deletions(-)
>> > ----------------------------------------------------------------------
>> >
>> >
>> >
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/9e970024/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> > ----------------------------------------------------------------------
>> > diff --git
>> a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> > index ea843c1..e625f38 100644
>> > ---
>> a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> > +++
>> b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> > @@ -52,11 +52,11 @@ import org.apache.wicket.validation.IValidator;
>> >   */
>> >  public class PropertyValidator<T> extends Behavior implements
>> IValidator<T>
>> >  {
>> > -       private static Class<?>[] EMPTY = new Class<?>[0];
>> > +       private static final Class<?>[] EMPTY = new Class<?>[0];
>> >
>> >         private FormComponent<T> component;
>> > -       private Property property_;
>> > -       private IModel<Class<?>[]> groups_;
>> > +       private Property property;
>> > +       private final IModel<Class<?>[]> groups;
>> >
>> >         public PropertyValidator(Class<?>... groups)
>> >         {
>> > @@ -75,16 +75,16 @@ public class PropertyValidator<T> extends Behavior
>> implements IValidator<T>
>> >
>> >         public PropertyValidator(Property property, IModel<Class<?>[]>
>> groups)
>> >         {
>> > -               this.property_ = property;
>> > -               this.groups_ = groups;
>> > +               this.property = property;
>> > +               this.groups = groups;
>> >         }
>> >
>> >         private Property getProperty()
>> >         {
>> > -               if (property_ == null)
>> > +               if (property == null)
>> >                 {
>> > -                       property_ =
>> BeanValidationConfiguration.get().resolveProperty(component);
>> > -                       if (property_ == null)
>> > +                       property =
>> BeanValidationConfiguration.get().resolveProperty(component);
>> > +                       if (property == null)
>> >                         {
>> >                                 throw new IllegalStateException(
>> >                                         "Could not resolve Property
>> from component: " +
>> > @@ -94,16 +94,16 @@ public class PropertyValidator<T> extends Behavior
>> implements IValidator<T>
>> >                                                 " to resolve the
>> Property automatically");
>> >                         }
>> >                 }
>> > -               return property_;
>> > +               return property;
>> >         }
>> >
>> >         private Class<?>[] getGroups()
>> >         {
>> > -               if (groups_ == null)
>> > +               if (groups == null)
>> >                 {
>> >                         return EMPTY;
>> >                 }
>> > -               return groups_.getObject();
>> > +               return groups.getObject();
>> >         }
>> >
>> >         @SuppressWarnings("unchecked")
>> > @@ -123,7 +123,7 @@ public class PropertyValidator<T> extends Behavior
>> implements IValidator<T>
>> >                                 " can only be added to FormComponents");
>> >                 }
>> >
>> > -               // TODO add a validation key that appends the type so
>> we can have differnet messages for
>> > +               // TODO add a validation key that appends the type so
>> we can have different messages for
>> >                 // @Size on String vs Collection - done but need to add
>> a key for each superclass/interface
>> >
>> >                 this.component = (FormComponent<T>)component;
>> > @@ -136,9 +136,9 @@ public class PropertyValidator<T> extends Behavior
>> implements IValidator<T>
>> >         public void detach(Component component)
>> >         {
>> >                 super.detach(component);
>> > -               if (groups_ != null)
>> > +               if (groups != null)
>> >                 {
>> > -                       groups_.detach();
>> > +                       groups.detach();
>> >                 }
>> >         }
>> >
>> >
>>
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>
>
>


-- 
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>

Re: [2/2] git commit: Make 'groups' member final and rename other field members to not have '_' as suffix to make them consistent with rest Wicket classes.

Posted by Igor Vaynberg <ig...@gmail.com>.
i think both should be used. the underscore to distract you - so you
will go check the comment/javadoc of the field.

-igor

On Sun, Nov 25, 2012 at 12:50 PM, Martin Grigorov <mg...@apache.org> wrote:
> I think comments/javadoc in the code would be better.
> The trailing underscore didn't tell me that this is the reason.
>
> If we are going to use them from now on then let's make it public in dev@.
>
>
> On Sun, Nov 25, 2012 at 9:30 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> can you please rename the fields back.
>>
>> the underscore at the end of the field means those fields should not
>> be accessed directly
>>
>> for example property_ should not be accessed directly - only via
>> getProperty() getter because it is lazily initialized. even though it
>> is private people modifying the code may not be aware of or not
>> remember not to access it directly.
>>
>> -igor
>>
>> On Sun, Nov 25, 2012 at 7:49 AM,  <mg...@apache.org> wrote:
>> > Make 'groups' member final and rename other field members to not have
>> '_' as suffix to make them consistent with rest Wicket classes.
>> >
>> >
>> > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/9e970024
>> > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/9e970024
>> > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/9e970024
>> >
>> > Branch: refs/heads/master
>> > Commit: 9e9700241eadf93bed2d2a4d61e7cadb998005d5
>> > Parents: ddf53c9
>> > Author: Martin Tzvetanov Grigorov <mg...@apache.org>
>> > Authored: Sun Nov 25 16:43:05 2012 +0100
>> > Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
>> > Committed: Sun Nov 25 16:43:05 2012 +0100
>> >
>> > ----------------------------------------------------------------------
>> >  .../wicket/bean/validation/PropertyValidator.java  |   28 +++++++-------
>> >  1 files changed, 14 insertions(+), 14 deletions(-)
>> > ----------------------------------------------------------------------
>> >
>> >
>> >
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/9e970024/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> > ----------------------------------------------------------------------
>> > diff --git
>> a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> > index ea843c1..e625f38 100644
>> > ---
>> a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> > +++
>> b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
>> > @@ -52,11 +52,11 @@ import org.apache.wicket.validation.IValidator;
>> >   */
>> >  public class PropertyValidator<T> extends Behavior implements
>> IValidator<T>
>> >  {
>> > -       private static Class<?>[] EMPTY = new Class<?>[0];
>> > +       private static final Class<?>[] EMPTY = new Class<?>[0];
>> >
>> >         private FormComponent<T> component;
>> > -       private Property property_;
>> > -       private IModel<Class<?>[]> groups_;
>> > +       private Property property;
>> > +       private final IModel<Class<?>[]> groups;
>> >
>> >         public PropertyValidator(Class<?>... groups)
>> >         {
>> > @@ -75,16 +75,16 @@ public class PropertyValidator<T> extends Behavior
>> implements IValidator<T>
>> >
>> >         public PropertyValidator(Property property, IModel<Class<?>[]>
>> groups)
>> >         {
>> > -               this.property_ = property;
>> > -               this.groups_ = groups;
>> > +               this.property = property;
>> > +               this.groups = groups;
>> >         }
>> >
>> >         private Property getProperty()
>> >         {
>> > -               if (property_ == null)
>> > +               if (property == null)
>> >                 {
>> > -                       property_ =
>> BeanValidationConfiguration.get().resolveProperty(component);
>> > -                       if (property_ == null)
>> > +                       property =
>> BeanValidationConfiguration.get().resolveProperty(component);
>> > +                       if (property == null)
>> >                         {
>> >                                 throw new IllegalStateException(
>> >                                         "Could not resolve Property from
>> component: " +
>> > @@ -94,16 +94,16 @@ public class PropertyValidator<T> extends Behavior
>> implements IValidator<T>
>> >                                                 " to resolve the
>> Property automatically");
>> >                         }
>> >                 }
>> > -               return property_;
>> > +               return property;
>> >         }
>> >
>> >         private Class<?>[] getGroups()
>> >         {
>> > -               if (groups_ == null)
>> > +               if (groups == null)
>> >                 {
>> >                         return EMPTY;
>> >                 }
>> > -               return groups_.getObject();
>> > +               return groups.getObject();
>> >         }
>> >
>> >         @SuppressWarnings("unchecked")
>> > @@ -123,7 +123,7 @@ public class PropertyValidator<T> extends Behavior
>> implements IValidator<T>
>> >                                 " can only be added to FormComponents");
>> >                 }
>> >
>> > -               // TODO add a validation key that appends the type so we
>> can have differnet messages for
>> > +               // TODO add a validation key that appends the type so we
>> can have different messages for
>> >                 // @Size on String vs Collection - done but need to add
>> a key for each superclass/interface
>> >
>> >                 this.component = (FormComponent<T>)component;
>> > @@ -136,9 +136,9 @@ public class PropertyValidator<T> extends Behavior
>> implements IValidator<T>
>> >         public void detach(Component component)
>> >         {
>> >                 super.detach(component);
>> > -               if (groups_ != null)
>> > +               if (groups != null)
>> >                 {
>> > -                       groups_.detach();
>> > +                       groups.detach();
>> >                 }
>> >         }
>> >
>> >
>>
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>

Re: [2/2] git commit: Make 'groups' member final and rename other field members to not have '_' as suffix to make them consistent with rest Wicket classes.

Posted by Martin Grigorov <mg...@apache.org>.
I think comments/javadoc in the code would be better.
The trailing underscore didn't tell me that this is the reason.

If we are going to use them from now on then let's make it public in dev@.


On Sun, Nov 25, 2012 at 9:30 PM, Igor Vaynberg <ig...@gmail.com>wrote:

> can you please rename the fields back.
>
> the underscore at the end of the field means those fields should not
> be accessed directly
>
> for example property_ should not be accessed directly - only via
> getProperty() getter because it is lazily initialized. even though it
> is private people modifying the code may not be aware of or not
> remember not to access it directly.
>
> -igor
>
> On Sun, Nov 25, 2012 at 7:49 AM,  <mg...@apache.org> wrote:
> > Make 'groups' member final and rename other field members to not have
> '_' as suffix to make them consistent with rest Wicket classes.
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/9e970024
> > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/9e970024
> > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/9e970024
> >
> > Branch: refs/heads/master
> > Commit: 9e9700241eadf93bed2d2a4d61e7cadb998005d5
> > Parents: ddf53c9
> > Author: Martin Tzvetanov Grigorov <mg...@apache.org>
> > Authored: Sun Nov 25 16:43:05 2012 +0100
> > Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
> > Committed: Sun Nov 25 16:43:05 2012 +0100
> >
> > ----------------------------------------------------------------------
> >  .../wicket/bean/validation/PropertyValidator.java  |   28 +++++++-------
> >  1 files changed, 14 insertions(+), 14 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/wicket/blob/9e970024/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> > ----------------------------------------------------------------------
> > diff --git
> a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> > index ea843c1..e625f38 100644
> > ---
> a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> > +++
> b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
> > @@ -52,11 +52,11 @@ import org.apache.wicket.validation.IValidator;
> >   */
> >  public class PropertyValidator<T> extends Behavior implements
> IValidator<T>
> >  {
> > -       private static Class<?>[] EMPTY = new Class<?>[0];
> > +       private static final Class<?>[] EMPTY = new Class<?>[0];
> >
> >         private FormComponent<T> component;
> > -       private Property property_;
> > -       private IModel<Class<?>[]> groups_;
> > +       private Property property;
> > +       private final IModel<Class<?>[]> groups;
> >
> >         public PropertyValidator(Class<?>... groups)
> >         {
> > @@ -75,16 +75,16 @@ public class PropertyValidator<T> extends Behavior
> implements IValidator<T>
> >
> >         public PropertyValidator(Property property, IModel<Class<?>[]>
> groups)
> >         {
> > -               this.property_ = property;
> > -               this.groups_ = groups;
> > +               this.property = property;
> > +               this.groups = groups;
> >         }
> >
> >         private Property getProperty()
> >         {
> > -               if (property_ == null)
> > +               if (property == null)
> >                 {
> > -                       property_ =
> BeanValidationConfiguration.get().resolveProperty(component);
> > -                       if (property_ == null)
> > +                       property =
> BeanValidationConfiguration.get().resolveProperty(component);
> > +                       if (property == null)
> >                         {
> >                                 throw new IllegalStateException(
> >                                         "Could not resolve Property from
> component: " +
> > @@ -94,16 +94,16 @@ public class PropertyValidator<T> extends Behavior
> implements IValidator<T>
> >                                                 " to resolve the
> Property automatically");
> >                         }
> >                 }
> > -               return property_;
> > +               return property;
> >         }
> >
> >         private Class<?>[] getGroups()
> >         {
> > -               if (groups_ == null)
> > +               if (groups == null)
> >                 {
> >                         return EMPTY;
> >                 }
> > -               return groups_.getObject();
> > +               return groups.getObject();
> >         }
> >
> >         @SuppressWarnings("unchecked")
> > @@ -123,7 +123,7 @@ public class PropertyValidator<T> extends Behavior
> implements IValidator<T>
> >                                 " can only be added to FormComponents");
> >                 }
> >
> > -               // TODO add a validation key that appends the type so we
> can have differnet messages for
> > +               // TODO add a validation key that appends the type so we
> can have different messages for
> >                 // @Size on String vs Collection - done but need to add
> a key for each superclass/interface
> >
> >                 this.component = (FormComponent<T>)component;
> > @@ -136,9 +136,9 @@ public class PropertyValidator<T> extends Behavior
> implements IValidator<T>
> >         public void detach(Component component)
> >         {
> >                 super.detach(component);
> > -               if (groups_ != null)
> > +               if (groups != null)
> >                 {
> > -                       groups_.detach();
> > +                       groups.detach();
> >                 }
> >         }
> >
> >
>



-- 
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>