You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lenya.apache.org by Andreas Hartmann <an...@apache.org> on 2006/08/14 09:41:17 UTC

Re: svn commit: r431178 - /lenya/trunk/src/modules-core/properties/java/src/org/apache/lenya/cms/cocoon/components/modules/input/PropertiesModule.java

Hi Thorsten,

thorsten@apache.org wrote:

> +    public Iterator getAttributeNames(Configuration modeConf, Map objectModel)
> +    throws ConfigurationException {
> +
> +SortedSet matchset = new TreeSet();
> +Enumeration enumeration = filteringProperties.keys();
> +while (enumeration.hasMoreElements()) {
> +    String key = (String) enumeration.nextElement();
> +    matchset.add(key);
> +}
> +Iterator iterator = super.getAttributeNames(modeConf, objectModel);
> +while (iterator.hasNext())
> +    matchset.add(iterator.next());
> +return matchset.iterator();
> +}

nag nag ... :)

Is this the real code, or did the SVN mail mess up the indentation?
And does the method have to be public?


>              filteringProperties = loadXMLPropertiesFromURI(filteringProperties,
>                      lenyaPropertiesStringURI);
> -        } catch (FileNotFoundException e) {
> -            if (debugging())
> -                debug("Unable to find local.lenya.properties.xml, ignoring.");
> -        }

Is this intended? IIUC the Exception is just swallowed in non-debug mode.
That looks like exception handling for flow control, which is a Bad Thing.
Shouldn't we check if the file exists instead?

-- Andreas



-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


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


Re: svn commit: r431178 - /lenya/trunk/src/modules-core/properties/java/src/org/apache/lenya/cms/cocoon/components/modules/input/PropertiesModule.java

Posted by Andreas Hartmann <an...@apache.org>.
Andreas Hartmann wrote:
> Hi Thorsten,
> 
> thorsten@apache.org wrote:
> 
>> +    public Iterator getAttributeNames(Configuration modeConf, Map 
>> objectModel)
>> +    throws ConfigurationException {
>> +
>> +SortedSet matchset = new TreeSet();
>> +Enumeration enumeration = filteringProperties.keys();
>> +while (enumeration.hasMoreElements()) {
>> +    String key = (String) enumeration.nextElement();
>> +    matchset.add(key);
>> +}
>> +Iterator iterator = super.getAttributeNames(modeConf, objectModel);
>> +while (iterator.hasNext())
>> +    matchset.add(iterator.next());
>> +return matchset.iterator();
>> +}

[...]

> And does the method have to be public?

Sorry, I didn't notice that it's part of the interface.
It seems strange to me to return an Iterator in an interface method, though ...

-- Andreas

-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


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


Re: svn commit: r431178 - /lenya/trunk/src/modules-core/properties/java/src/org/apache/lenya/cms/cocoon/components/modules/input/PropertiesModule.java

Posted by Andreas Hartmann <an...@apache.org>.
Thorsten Scherler wrote:

[...]

>>
>>>              filteringProperties = loadXMLPropertiesFromURI(filteringProperties,
>>>                      lenyaPropertiesStringURI);
>>> -        } catch (FileNotFoundException e) {
>>> -            if (debugging())
>>> -                debug("Unable to find local.lenya.properties.xml, ignoring.");
>>> -        }
>> Is this intended? 
> 
> Yes.
> 
>> IIUC the Exception is just swallowed in non-debug mode.
>> That looks like exception handling for flow control, which is a Bad Thing.
>> Shouldn't we check if the file exists instead?
> 
> See loadXMLPropertiesFromURI(...) there is a source.exist() that is why
> FileNotFoundException will not appear anymore. Since they do not appear
> it does not make sense to catch them (that is why I removed them).

I guess it was too early in the morning, I didn't pay attention to the - signs.
Sorry ...

> Why do you think we should keep it? 
> 
> Why was it there before?
> http://svn.apache.org/viewvc?rev=428447&view=rev
> 
> I agree with your opinion that is why I opened
> https://issues.apache.org/jira/browse/FOR-914 a week ago.

That's very good to know :)

-- Andreas


-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


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


Re: svn commit: r431178 - /lenya/trunk/src/modules-core/properties/java/src/org/apache/lenya/cms/cocoon/components/modules/input/PropertiesModule.java

Posted by Thorsten Scherler <th...@apache.org>.
El lun, 14-08-2006 a las 09:41 +0200, Andreas Hartmann escribió:
> Hi Thorsten,
> 
> thorsten@apache.org wrote:
> 
> > +    public Iterator getAttributeNames(Configuration modeConf, Map objectModel)
> > +    throws ConfigurationException {
> > +
> > +SortedSet matchset = new TreeSet();
> > +Enumeration enumeration = filteringProperties.keys();
> > +while (enumeration.hasMoreElements()) {
> > +    String key = (String) enumeration.nextElement();
> > +    matchset.add(key);
> > +}
> > +Iterator iterator = super.getAttributeNames(modeConf, objectModel);
> > +while (iterator.hasNext())
> > +    matchset.add(iterator.next());
> > +return matchset.iterator();
> > +}
> 
> nag nag ... :)
> 
> Is this the real code, or did the SVN mail mess up the indentation?

Nupp, I did not came around to format before commit. Then I already
added the code and did not want to mess up the diff with white noise. 

I forgot to clean up again. Will do so now. Thanks for pointing out.

> And does the method have to be public?

Like you found out it is the API that says so.

> 
> 
> >              filteringProperties = loadXMLPropertiesFromURI(filteringProperties,
> >                      lenyaPropertiesStringURI);
> > -        } catch (FileNotFoundException e) {
> > -            if (debugging())
> > -                debug("Unable to find local.lenya.properties.xml, ignoring.");
> > -        }
> 
> Is this intended? 

Yes.

> IIUC the Exception is just swallowed in non-debug mode.
> That looks like exception handling for flow control, which is a Bad Thing.
> Shouldn't we check if the file exists instead?

See loadXMLPropertiesFromURI(...) there is a source.exist() that is why
FileNotFoundException will not appear anymore. Since they do not appear
it does not make sense to catch them (that is why I removed them).

Why do you think we should keep it? 

Why was it there before?
http://svn.apache.org/viewvc?rev=428447&view=rev

I agree with your opinion that is why I opened
https://issues.apache.org/jira/browse/FOR-914 a week ago.

salu2

> 
> -- Andreas
> 
> 
> 
-- 
thorsten

"Together we stand, divided we fall!" 
Hey you (Pink Floyd)


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