Re: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/components/request RequestFactory.java

[Vadim Gritsenko <va...@verizon.net>]

stefano@apache.org wrote:

>stefano     2003/04/03 06:16:32
>
>  Modified:    src/java/org/apache/cocoon/components/request
>                        RequestFactory.java
>  Log:
>  removing fixmes and adding explainations  
>  
>
...

>           try {
>               ClassLoader loader = Thread.currentThread().getContextClassLoader();
>               Class clazz = loader.loadClass(className);
>  -            factory = (RequestFactory)clazz.newInstance();
>  +            factory = (RequestFactory) clazz.newInstance();
>           } catch (Throwable t) {
>  -            // FIXME (VG): Is it Ok to ignore all exceptions?
>  +            // the try/catch mechanism is used because there is no way
>  +            // to know if a classloader contains a class without asking for it
>           }
>  
>
...

>           if (factory == null) {
>               factory = new SimpleRequestFactoryImpl();
>           }
>

I think my comment was more about "Is it ok to return 
SimpleRequestFactoryImpl instead of requested factory (via paramater 
className) and not even print any warning about it".

Vadim

Re: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/components/request RequestFactory.java

[Stefano Mazzocchi <st...@apache.org>]

Vadim Gritsenko wrote:
> stefano@apache.org wrote:
> 
>> stefano     2003/04/03 06:16:32
>>
>>  Modified:    src/java/org/apache/cocoon/components/request
>>                        RequestFactory.java
>>  Log:
>>  removing fixmes and adding explainations   
>>
> ...
> 
>>           try {
>>               ClassLoader loader = 
>> Thread.currentThread().getContextClassLoader();
>>               Class clazz = loader.loadClass(className);
>>  -            factory = (RequestFactory)clazz.newInstance();
>>  +            factory = (RequestFactory) clazz.newInstance();
>>           } catch (Throwable t) {
>>  -            // FIXME (VG): Is it Ok to ignore all exceptions?
>>  +            // the try/catch mechanism is used because there is no way
>>  +            // to know if a classloader contains a class without 
>> asking for it
>>           }
>>  
>>
> ...
> 
>>           if (factory == null) {
>>               factory = new SimpleRequestFactoryImpl();
>>           }
>>
> 
> I think my comment was more about "Is it ok to return 
> SimpleRequestFactoryImpl instead of requested factory (via paramater 
> className) and not even print any warning about it".

Ah, well, you should have written that instead ;-)

no, seriously, in my local copy I'm removing the ability to have 
pluggable request factories and this is mostly because if you do, there 
is no way to make the upload facilities workable.

I've looked into the way the request factories work along with the 
environment and, well, it's a HUGE hack.

But I don't want to touch this before we release, so I'm making 
refactoring that doesn't alter the environment contracts (for now).

In the future, we really need to redesign the environment for better 
input friendlyness or we are doomed.

Anybody against this?

Stefano.