Re: copy properties for inheritance (Re: svn commit: r178550)
On 5/26/05, Martin Cooper <[EMAIL PROTECTED]> wrote: > > I'm not sure how you can be burned by not having direct access to a > member while there are getters and setters available. My mistake. The cases that keep flashing in my head provided getters (if even that) but no setters. In those cases, I had problems because the superclass would use the instance without going through the getter, so even if I overrode the getter, I couldn't make the change the behavior of the superclass. Hubert > On the other > hand, making a member directly accessable severely curbs the ability > to change the class implementation without breaking backwards > compatibility. Suppose at some point that you decide you don't want to > keep a reference to the member in the class, but retrieve it on the > fly. Or perhaps you decide that you want to keep the reference in a > cache of weak references. You're hosed, because any code that used > that member is now broken. > > This is something that I really dislike in a lot of the Struts code. > For much of it, it's no doubt too late to change. However, I'd very > much prefer that we don't continue to exacerbate the problem. > > -- > Martin Cooper > > - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
On 5/26/05, Hubert Rabago <[EMAIL PROTECTED]> wrote: > On 5/26/05, Riyaz Mansoor <[EMAIL PROTECTED]> wrote: > > Hubert Rabago wrote: > > > > my intention here was to have _all_ the accessor methods of "properties" > > in BaseConfig - i guess it can be a hinderance when the classes a > > changing rapidly as u've mentioned. i think of it as a 'cleaner' way :) > > > > That's what we have now, accessors of "properties" in BaseConfig. In > fact, that's the only way to access them, which goes against the > convention set by most (if not all) other config objects. Other > config objects declare their fields as protected, allowing custom > subclasses to access those fields directly. I'll probably change > "properties" in BaseConfig to follow this convention. > > > > > eg: pass the childConfig to the ParentConfig instead of the passing the > > parentConfig (baseConfig) to the childConfig. > > It took me a while to understand what you were saying here, but I > think I finally get it. I'm not opposed to the idea, but stepping > back, I'm not entirely convinced that one is cleaner than the other. > We've been discussing this only from the perspective of copying > arbitrary properties of config objects held by an ActionConfig. The > rest of the config objects have been doing > "child-inherits-from-parent" as well. If "parent-sets-child-values" > is truly that much better, I'd want to use that for the rest of the > config objects. Not just when copying full config, but also when > extending overridden configs. > > > > > again, extending classes would have limited access to the properties object. > > This point is independent of "parent-sets-child-values" vs. > "child-inherits-from-parent". Whichever wins out there, doesn't > convince me that superclasses should forbid subclasses from accessing > fields. > > > > > it comes down to the first point u made. :) ie: > > > > > I'm not sure I want to forbid config subclasses from accessing > > > their properties directly. I've come across several libraries > > > and frameworks which used "private" all throughout and made it > > > difficult if not impossible for me to customize certain behavior. > > > > different ways of thinking ;) > > > > Yes, but it doesn't mean I'm not open to other ideas. In this case, > I started out reading from everywhere that fields should be private, > and everything should go through accessors (even subclasses). But I've > been burned by that restriction LOTs of times (in some cases, I had > to rebuild entire libraries to get access to the fields I needed), so > I'll need some convincing. If private fields are what's best, how come > every other config field is protected? I'm not sure how you can be burned by not having direct access to a member while there are getters and setters available. On the other hand, making a member directly accessable severely curbs the ability to change the class implementation without breaking backwards compatibility. Suppose at some point that you decide you don't want to keep a reference to the member in the class, but retrieve it on the fly. Or perhaps you decide that you want to keep the reference in a cache of weak references. You're hosed, because any code that used that member is now broken. This is something that I really dislike in a lot of the Struts code. For much of it, it's no doubt too late to change. However, I'd very much prefer that we don't continue to exacerbate the problem. -- Martin Cooper > Hubert > > > riyaz > > > > - > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
If I understood what you're saying, then yes, that's how all other config fields are, and I'll probably change 'properties' to conform to that (unless someone else beats me to it). Hubert On 5/26/05, Riyaz Mansoor <[EMAIL PROTECTED]> wrote: > Hubert Rabago wrote: > > > Yes, but it doesn't mean I'm not open to other ideas. In this case, > > I started out reading from everywhere that fields should be private, > > and everything should go through accessors (even subclasses). But I've > > been burned by that restriction LOTs of times (in some cases, I had > > to rebuild entire libraries to get access to the fields I needed), so > > I'll need some convincing. If private fields are what's best, how come > > every other config field is protected? > > i suppose a middle ground would be to keep the properties object > protected but provide and use only accessor methods within struts? > > > :) > > riyaz > - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
Hubert Rabago wrote: Yes, but it doesn't mean I'm not open to other ideas. In this case, I started out reading from everywhere that fields should be private, and everything should go through accessors (even subclasses). But I've been burned by that restriction LOTs of times (in some cases, I had to rebuild entire libraries to get access to the fields I needed), so I'll need some convincing. If private fields are what's best, how come every other config field is protected? i suppose a middle ground would be to keep the properties object protected but provide and use only accessor methods within struts? :) riyaz - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
Hubert Rabago wrote: Yes, but it doesn't mean I'm not open to other ideas. In this case, I started out reading from everywhere that fields should be private, and everything should go through accessors (even subclasses). But I've been burned by that restriction LOTs of times (in some cases, I had to rebuild entire libraries to get access to the fields I needed), so I'll need some convincing. If private fields are what's best, how come every other config field is protected? i suppose a middle ground would be to keep the properties object protected but provide and use only accessor methods within struts? :) riyaz - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
On 5/26/05, Riyaz Mansoor <[EMAIL PROTECTED]> wrote: > > at the moment the getProperties() returns the Map object. no necessity > for the 2 accessor methods getProperty,setProperty > The getProperties()/setProperties() accessors are protected and are for subclasses. The getProperty()/setProperty() accessors are public, for use by everyone else. Hubert - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
Hubert Rabago wrote: That's what we have now, accessors of "properties" in BaseConfig. In fact, that's the only way to access them, which goes against the convention set by most (if not all) other config objects. Other config objects declare their fields as protected, allowing custom subclasses to access those fields directly. I'll probably change "properties" in BaseConfig to follow this convention. at the moment the getProperties() returns the Map object. no necessity for the 2 accessor methods getProperty,setProperty Yes, but it doesn't mean I'm not open to other ideas. In this case, I started out reading from everywhere that fields should be private, and everything should go through accessors (even subclasses). But I've been burned by that restriction LOTs of times (in some cases, I had to rebuild entire libraries to get access to the fields I needed), so I'll need some convincing. If private fields are what's best, how come every other config field is protected? don't know about that :) but yes the "officially" (if it can be called that) seems to be to make fields private and provide accessor methods. riyaz - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
On 5/26/05, Riyaz Mansoor <[EMAIL PROTECTED]> wrote: > Hubert Rabago wrote: > > my intention here was to have _all_ the accessor methods of "properties" > in BaseConfig - i guess it can be a hinderance when the classes a > changing rapidly as u've mentioned. i think of it as a 'cleaner' way :) > That's what we have now, accessors of "properties" in BaseConfig. In fact, that's the only way to access them, which goes against the convention set by most (if not all) other config objects. Other config objects declare their fields as protected, allowing custom subclasses to access those fields directly. I'll probably change "properties" in BaseConfig to follow this convention. > > eg: pass the childConfig to the ParentConfig instead of the passing the > parentConfig (baseConfig) to the childConfig. It took me a while to understand what you were saying here, but I think I finally get it. I'm not opposed to the idea, but stepping back, I'm not entirely convinced that one is cleaner than the other. We've been discussing this only from the perspective of copying arbitrary properties of config objects held by an ActionConfig. The rest of the config objects have been doing "child-inherits-from-parent" as well. If "parent-sets-child-values" is truly that much better, I'd want to use that for the rest of the config objects. Not just when copying full config, but also when extending overridden configs. > > again, extending classes would have limited access to the properties object. This point is independent of "parent-sets-child-values" vs. "child-inherits-from-parent". Whichever wins out there, doesn't convince me that superclasses should forbid subclasses from accessing fields. > > it comes down to the first point u made. :) ie: > > > I'm not sure I want to forbid config subclasses from accessing > > their properties directly. I've come across several libraries > > and frameworks which used "private" all throughout and made it > > difficult if not impossible for me to customize certain behavior. > > different ways of thinking ;) > Yes, but it doesn't mean I'm not open to other ideas. In this case, I started out reading from everywhere that fields should be private, and everything should go through accessors (even subclasses). But I've been burned by that restriction LOTs of times (in some cases, I had to rebuild entire libraries to get access to the fields I needed), so I'll need some convincing. If private fields are what's best, how come every other config field is protected? Hubert > riyaz > - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
Hubert Rabago wrote: I'm not sure I want to forbid config subclasses from accessing their properties directly. I've come across several libraries and frameworks which used "private" all throughout and made it difficult if not impossible for me to customize certain behavior. (I'm still running into those issues with the app I inherited here in my day job.) The 'properties' field was initially declared as protected when it was only in ActionConfig. The goal was to reduce the likelihood that subclasses would be needed. Now that it's in a class that's intended to be subclassed, I wonder why it became private. Joe? there is a getProperties() method which retrieves the "properties" object my intention here was to have _all_ the accessor methods of "properties" in BaseConfig - i guess it can be a hinderance when the classes a changing rapidly as u've mentioned. i think of it as a 'cleaner' way :) classes, only ActionConfig uses "properties" and needs to change for this - not much. We also need it in FormBeanConfig. :) BaseConfig already has an "inheritProperties(BaseConfig)" method, doing just what you've outlined. This is used by subclasses when extending another config object, not to be confused with the need to "copy" another config object, which is when ActionConfig and FormBeanConfig needs access to the properties field. yes, very recently added :). again, what am trying to say is that instead of moving the "properties" object (using getProperties()) the properties can be inherited without retrieving the whole "properties" object. eg: pass the childConfig to the ParentConfig instead of the passing the parentConfig (baseConfig) to the childConfig. again, extending classes would have limited access to the properties object. it comes down to the first point u made. :) ie: > I'm not sure I want to forbid config subclasses from accessing > their properties directly. I've come across several libraries > and frameworks which used "private" all throughout and made it > difficult if not impossible for me to customize certain behavior. different ways of thinking ;) riyaz - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
On 5/25/05, Riyaz Mansoor <[EMAIL PROTECTED]> wrote:
>
> just a thought.
>
> ie, not expose the "properties" object (as it is) by the
> getProperties,setProperties methods.
I'm not sure I want to forbid config subclasses from accessing
their properties directly. I've come across several libraries
and frameworks which used "private" all throughout and made it
difficult if not impossible for me to customize certain behavior.
(I'm still running into those issues with the app I inherited
here in my day job.)
The 'properties' field was initially declared as protected when
it was only in ActionConfig. The goal was to reduce the likelihood
that subclasses would be needed. Now that it's in a class that's
intended to be subclassed, I wonder why it became private. Joe?
> by my last viewing of the Config
> classes, only ActionConfig uses "properties" and needs to change for
> this - not much.
We also need it in FormBeanConfig.
> get/setProperties can be replaced by
>
> void inheritProperties(childConfig) {
> // for each "xx" property in (parent) class
> // if childConfig.getProperty("xx") == null
> // copy the arbitrary property
> }
>
>
> imho, i think this is a good way to do this, provides separation between
> BaseConfig and its extending classes
BaseConfig already has an "inheritProperties(BaseConfig)" method, doing just
what you've outlined. This is used by subclasses when extending another
config object, not to be confused with the need to "copy" another config
object, which is when ActionConfig and FormBeanConfig needs access to the
properties field.
Hubert
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: copy properties for inheritance (Re: svn commit: r178550)
On 5/25/05, Joe Germuska <[EMAIL PROTECTED]> wrote:
> I was wondering about this:
>
> > BeanUtils.copyProperties(copy, baseHandler);
> > this.addExceptionConfig(copy);
> >-
> >+copy.setProperties(baseHandler.getProperties());
> >+
>
> doesn't the strategy you implemented clobber properties which are
> set on the extending/copy? Shouldn't this process somehow honor
> properties which were already set in the copy? I think it should go
> through the list of property names in the baseHandler and, where the
> property is not already defined in the copy, in those cases it should
> copy them.
>
This part of the code executes when the extending config object
(ActionConfig in this case) isn't overriding baseHandler:
// Do we have this handler?
ExceptionConfig copy =
this.findExceptionConfig(baseHandler.getType());
if (copy == null) {
// We don't have this, so let's copy it
copy = (ExceptionConfig) RequestUtils
.applicationInstance(baseHandler.getClass().getName());
BeanUtils.copyProperties(copy, baseHandler);
this.addExceptionConfig(copy);
copy.setProperties(baseHandler.getProperties());
} else {
// process any extension that this config might have
copy.processExtends(getModuleConfig(), this);
}
Hubert
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
copy properties for inheritance (Re: svn commit: r178550)
I was wondering about this: BeanUtils.copyProperties(copy, baseHandler); this.addExceptionConfig(copy); - +copy.setProperties(baseHandler.getProperties()); + doesn't the strategy you implemented clobber properties which are set on the extending/copy? Shouldn't this process somehow honor properties which were already set in the copy? I think it should go through the list of property names in the baseHandler and, where the property is not already defined in the copy, in those cases it should copy them. I haven't studied this, so forgive me if I'm overlooking something Joe -- Joe Germuska [EMAIL PROTECTED] http://blog.germuska.com "Narrow minds are weapons made for mass destruction" -The Ex - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
