You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Stefan Bodewig <bo...@apache.org> on 2008/11/14 17:29:19 UTC

Re: svn commit: r714053 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/MappedResource.java

I need help!

I'm trying to write a mapped-resource that is a decorator for a
different resource and changes the name of the decorated resource
using a mapper.

Resource itself is a class, so my base class is set.  There are
several "optional" behaviors available via interfaces, namely
Touchable, Appendable and FileProvider (I hope I didn't miss one).

Our tasks use instanceof checks in several places, so if the mapped
resource wraps a FileResource it should also implement all three of
those interfaces.  OTOH it must not implement any of these interfaces
if wrapping a StringResource.

Any attempt of using anonymous subclasses of MappedResource or
reflection proxies leads to objects that either no longer extend
Resource or contain the methods required by an interface but don't
implement it.

The current solution is really really ugly (seven subclasses with many
of them identical except for the class they extend):

On 2008-11-14, <bo...@apache.org> wrote:

>>    public static MappedResource map(Resource r) {
>>        if (r instanceof FileProvider) {
>>            if (r instanceof Touchable) {
>>                if (r instanceof Appendable) {
>>                    // most probably FileResource
>>                    return new AppendableTouchableFileProviderMR(r);
>>                }
>>                return new TouchableFileProviderMR(r);
>>            }
>>            if (r instanceof Appendable) {
>>                return new AppendableFileProviderMR(r);
>>            }
>>            return new FileProviderMR(r);
>>        }
>>        if (r instanceof Touchable) {
>>            if (r instanceof Appendable) {
>>                return new AppendableTouchableMR(r);
>>            }
>>            return new TouchableMR(r);
>>        }
>>        if (r instanceof Appendable) {
>>            return new AppendableMR(r);
>>        }
>>        // no special interface
>>        return new MappedResource(r);
>>    }

The other approach I could take is to clone the wrapped resource (all
our resources are cloneable) and change the clone's name - and not
have a decorator at all.

The main problem with this approach is that some resources are
immutable and will throw an exception in setName (StringResource for
example) while others will not do what I want - FileResource will
simply ignore setName().

Any other ideas (apart from not using Java?)

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r714053 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/MappedResource.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2008-11-14, Bruce Atherton <br...@callenish.com> wrote:

> I haven't taken a look at the the code so this is off the cuff, but
> couldn't you use a boolean field for each of the interfaces on the
> Resource class to indicate whether it is supported?

That was my first approach.

> Then have any methods that implement that interface check the
> boolean flag to see whether to proxy the request to the decorated
> resource or to throw an exception. In code which needs to determine
> the abilities of any resource, it can call an is() method rather
> than using instanceof.

That's the big point, I'd need to modify a lot of tasks.

Maybe I still should use that approach for Appendable and Touchable as
those are pretty rarely used and stick to a single sub-class that
implements FileProvider which may even be used by third-party tasks.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r714053 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/MappedResource.java

Posted by Bruce Atherton <br...@callenish.com>.
I haven't taken a look at the the code so this is off the cuff, but 
couldn't you use a boolean field for each of the interfaces on the 
Resource class to indicate whether it is supported? Then have any 
methods that implement that interface check the boolean flag to see 
whether to proxy the request to the decorated resource or to throw an 
exception. In code which needs to determine the abilities of any 
resource, it can call an is() method rather than using instanceof. The 
actual resource implementations would typically hard code the is() 
return values and ignore or not implement the set() methods.

Something like:

public static MappedResource map(Resource r) {
    MappedResource decorator = new MappedResource(r);
    decorator.setTouchable(r.isTouchable());
    decorator.setAppendable(r.isAppendable());
    decorator.setFileProvider(r.isFileProvider());
    return decorator;
}


Stefan Bodewig wrote:
> I need help!
>
> I'm trying to write a mapped-resource that is a decorator for a
> different resource and changes the name of the decorated resource
> using a mapper.
>
> Resource itself is a class, so my base class is set.  There are
> several "optional" behaviors available via interfaces, namely
> Touchable, Appendable and FileProvider (I hope I didn't miss one).
>
> Our tasks use instanceof checks in several places, so if the mapped
> resource wraps a FileResource it should also implement all three of
> those interfaces.  OTOH it must not implement any of these interfaces
> if wrapping a StringResource.
>
> Any attempt of using anonymous subclasses of MappedResource or
> reflection proxies leads to objects that either no longer extend
> Resource or contain the methods required by an interface but don't
> implement it.
>
> The current solution is really really ugly (seven subclasses with many
> of them identical except for the class they extend):
>
> On 2008-11-14, <bo...@apache.org> wrote:
>
>   
>>>    public static MappedResource map(Resource r) {
>>>        if (r instanceof FileProvider) {
>>>            if (r instanceof Touchable) {
>>>                if (r instanceof Appendable) {
>>>                    // most probably FileResource
>>>                    return new AppendableTouchableFileProviderMR(r);
>>>                }
>>>                return new TouchableFileProviderMR(r);
>>>            }
>>>            if (r instanceof Appendable) {
>>>                return new AppendableFileProviderMR(r);
>>>            }
>>>            return new FileProviderMR(r);
>>>        }
>>>        if (r instanceof Touchable) {
>>>            if (r instanceof Appendable) {
>>>                return new AppendableTouchableMR(r);
>>>            }
>>>            return new TouchableMR(r);
>>>        }
>>>        if (r instanceof Appendable) {
>>>            return new AppendableMR(r);
>>>        }
>>>        // no special interface
>>>        return new MappedResource(r);
>>>    }
>>>       
>
> The other approach I could take is to clone the wrapped resource (all
> our resources are cloneable) and change the clone's name - and not
> have a decorator at all.
>
> The main problem with this approach is that some resources are
> immutable and will throw an exception in setName (StringResource for
> example) while others will not do what I want - FileResource will
> simply ignore setName().
>
> Any other ideas (apart from not using Java?)
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>   


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r714053 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/MappedResource.java

Posted by Nicolas Lalevée <ni...@hibnet.org>.
Stefan Bodewig a écrit :
> On 2008-11-17, Nicolas Lalevée <ni...@hibnet.org> wrote:
>
>   
>> Stefan Bodewig a écrit :
>>     
>>> On 2008-11-14, Nicolas Lalevée <ni...@hibnet.org> wrote:
>>>       
>
>
>   
>>>> But I see another solution, quite different. Probably most of the work
>>>> should be done in the Resource class:
>>>>         
>
>   
>>> I thought about that, but Resource is already doing way too much IMHO.
>>>       
>
>   
>> Well, in fact here you want a functionality to Resource (AFAIU).
>>     
>
> No.
>
> In the end what I want is a real decorator that attaches mapping on
> top of an existing resource collection.  Something that allows people
> to write
>
> <war ...
>   <mappedresource>
>     <path refid="some-path"/>
>     <chainedmapper>
>       <flattenmapper/>
>       <globmapper from="*" to="WEB-INF/lib/*"/>
>     </chainedmapper>
>   </mappedresource>
> </war>
>
> to package up all jars from nan arbitrary path into a WAR's lib
> directory.
>
> This really is pure decoration and not a functionality of the
> resources involved.
>   
ok, I understand now. So this makes sense to use a decorator pattern.


Nicolas, having his first look in ant code :)

>   
>> If you choose the former solution, then each time you want to add
>> another implementation of Resource with some new capability, you will
>> have to support it in MappedResource.
>>     
>
> MappedResource now delegates on the as() (what is getAdapter() in
> Eclipse) on to the decorated resource, no reason to support any of the
> exiting interfaces.
>
>   
>> By the way, what about the Resource implementation in some ant
>> plugins ? Is the Resource API exported ?
>>     
>
> Yes.
>
>   
>> Wouldn't the former solution break some ant plugin ?
>>     
>
> I lost track what "the former" was.  I do expect that people are doing
> instanceof checks on FileProvider (or more likely on FileResource -
> but then we are out of luck anyway), that's why I kept the
> FileProvider implementing subclass.
>
> Appendable is new and the use cases I see for MappedResource make it
> unlikely that you'd use it in a context where Touchable makes sense.
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>   


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r714053 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/MappedResource.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2008-11-17, Nicolas Lalevée <ni...@hibnet.org> wrote:

> Stefan Bodewig a écrit :
>> On 2008-11-14, Nicolas Lalevée <ni...@hibnet.org> wrote:


>>> But I see another solution, quite different. Probably most of the work
>>> should be done in the Resource class:

>> I thought about that, but Resource is already doing way too much IMHO.

> Well, in fact here you want a functionality to Resource (AFAIU).

No.

In the end what I want is a real decorator that attaches mapping on
top of an existing resource collection.  Something that allows people
to write

<war ...
  <mappedresource>
    <path refid="some-path"/>
    <chainedmapper>
      <flattenmapper/>
      <globmapper from="*" to="WEB-INF/lib/*"/>
    </chainedmapper>
  </mappedresource>
</war>

to package up all jars from nan arbitrary path into a WAR's lib
directory.

This really is pure decoration and not a functionality of the
resources involved.

> If you choose the former solution, then each time you want to add
> another implementation of Resource with some new capability, you will
> have to support it in MappedResource.

MappedResource now delegates on the as() (what is getAdapter() in
Eclipse) on to the decorated resource, no reason to support any of the
exiting interfaces.

> By the way, what about the Resource implementation in some ant
> plugins ? Is the Resource API exported ?

Yes.

> Wouldn't the former solution break some ant plugin ?

I lost track what "the former" was.  I do expect that people are doing
instanceof checks on FileProvider (or more likely on FileResource -
but then we are out of luck anyway), that's why I kept the
FileProvider implementing subclass.

Appendable is new and the use cases I see for MappedResource make it
unlikely that you'd use it in a context where Touchable makes sense.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r714053 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/MappedResource.java

Posted by Nicolas Lalevée <ni...@hibnet.org>.
Stefan Bodewig a écrit :
> On 2008-11-14, Nicolas Lalevée <ni...@hibnet.org> wrote:
>
>   
>> I am not sure of how are used the "Resource" in the Ant code, but it
>> makes me think of the Eclipse adapters (probably a design pattern ?):
>> http://www.jeffreyricker.com/papers/Eclipse-Adapters.pdf
>>     
>
> Sounds reasonable.
>
>   
>> Though it changes the tests of the appendability, the touchability,
>> etc... of a Resource in the ant code, the "instanceof+cast" would be
>> replaced by some "getAdapter". This solution is not very self-
>> contained...
>>     
>
> Yes.  I may stick a single subclass for FileProvider resources and
> modify all occurances of Appendable and Touchable to use the adapter
> method.
>
>
>   
>> But I see another solution, quite different. Probably most of the work
>> should be done in the Resource class:
>>     
>
> I thought about that, but Resource is already doing way too much IMHO.
>   
Well, in fact here you want a functionality to Resource (AFAIU). And it 
makes me think of the multi-inheritance issue. And after a long debate 
with a colleague of mine, he convinced me that multi-inheritance can 
always be resolved with single-inheritance + composition: extract the 
functionality into its own class and make it a composition of the main 
class. Even more, since I understood it, my code began very modular 
actually, even if it began to be composed of a lot of handler.handle().

If you choose the former solution, then each time you want to add 
another implementation of Resource with some new capability, you will 
have to support it in MappedResource. By the way, what about the 
Resource implementation in some ant plugins ? Is the Resource API 
exported ? Wouldn't the former solution break some ant plugin ?

Nicolas


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r714053 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/MappedResource.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2008-11-14, Nicolas Lalevée <ni...@hibnet.org> wrote:

> I am not sure of how are used the "Resource" in the Ant code, but it
> makes me think of the Eclipse adapters (probably a design pattern ?):
> http://www.jeffreyricker.com/papers/Eclipse-Adapters.pdf

Sounds reasonable.

> Though it changes the tests of the appendability, the touchability,
> etc... of a Resource in the ant code, the "instanceof+cast" would be
> replaced by some "getAdapter". This solution is not very self-
> contained...

Yes.  I may stick a single subclass for FileProvider resources and
modify all occurances of Appendable and Touchable to use the adapter
method.


> But I see another solution, quite different. Probably most of the work
> should be done in the Resource class:

I thought about that, but Resource is already doing way too much IMHO.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r714053 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/MappedResource.java

Posted by Nicolas Lalevée <ni...@hibnet.org>.
I am not sure of how are used the "Resource" in the Ant code, but it  
makes me think of the Eclipse adapters (probably a design pattern ?):
http://www.jeffreyricker.com/papers/Eclipse-Adapters.pdf

Though it changes the tests of the appendability, the touchability,  
etc... of a Resource in the ant code, the "instanceof+cast" would be  
replaced by some "getAdapter". This solution is not very self- 
contained...

But I see another solution, quite different. Probably most of the work  
should be done in the Resource class:

interface NameMapper {
   String getName();
}

public class Resource extends DataType implements .....
   NameMapper nameMapper = new NameMapper() {
     return isReference() ? ((Resource) getCheckedRef()).getName() :  
name;
   }
   public String getName() {
     return nameMapper.getName();
   }
}

And then:

public static Resource map(Resource r) {
   r. nameMapper = new NameMapper() {
     return doMapName();
   }
}


hoping it helps,
Nicolas



Le 14 nov. 08 à 17:29, Stefan Bodewig a écrit :

> I need help!
>
> I'm trying to write a mapped-resource that is a decorator for a
> different resource and changes the name of the decorated resource
> using a mapper.
>
> Resource itself is a class, so my base class is set.  There are
> several "optional" behaviors available via interfaces, namely
> Touchable, Appendable and FileProvider (I hope I didn't miss one).
>
> Our tasks use instanceof checks in several places, so if the mapped
> resource wraps a FileResource it should also implement all three of
> those interfaces.  OTOH it must not implement any of these interfaces
> if wrapping a StringResource.
>
> Any attempt of using anonymous subclasses of MappedResource or
> reflection proxies leads to objects that either no longer extend
> Resource or contain the methods required by an interface but don't
> implement it.
>
> The current solution is really really ugly (seven subclasses with many
> of them identical except for the class they extend):
>
> On 2008-11-14, <bo...@apache.org> wrote:
>
>>>   public static MappedResource map(Resource r) {
>>>       if (r instanceof FileProvider) {
>>>           if (r instanceof Touchable) {
>>>               if (r instanceof Appendable) {
>>>                   // most probably FileResource
>>>                   return new AppendableTouchableFileProviderMR(r);
>>>               }
>>>               return new TouchableFileProviderMR(r);
>>>           }
>>>           if (r instanceof Appendable) {
>>>               return new AppendableFileProviderMR(r);
>>>           }
>>>           return new FileProviderMR(r);
>>>       }
>>>       if (r instanceof Touchable) {
>>>           if (r instanceof Appendable) {
>>>               return new AppendableTouchableMR(r);
>>>           }
>>>           return new TouchableMR(r);
>>>       }
>>>       if (r instanceof Appendable) {
>>>           return new AppendableMR(r);
>>>       }
>>>       // no special interface
>>>       return new MappedResource(r);
>>>   }
>
> The other approach I could take is to clone the wrapped resource (all
> our resources are cloneable) and change the clone's name - and not
> have a decorator at all.
>
> The main problem with this approach is that some resources are
> immutable and will throw an exception in setName (StringResource for
> example) while others will not do what I want - FileResource will
> simply ignore setName().
>
> Any other ideas (apart from not using Java?)
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org