cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vadim Gritsenko <va...@reverycodes.com>
Subject Re: FIXME in IncludeTransformer [Was: Re: Adding Files to Subversion]
Date Tue, 28 Sep 2004 18:10:48 GMT
Pier Fumagalli wrote:

> On 28 Sep 2004, at 18:13, Vadim Gritsenko wrote:
> 
>> Pier Fumagalli wrote:
>>
>>> On 28 Sep 2004, at 12:00, Vadim Gritsenko wrote:
>>>
>>>> Go ahead, add parameters.
>>
>> ...
>>
>>>>   <i:parameter name="name">value</i:parameter>
>>>
>>> Done, I'm posting the patch here before applying just to triple-check 
>>>  I'm not f***ing up the whole thing. I mean, it works for me, but do 
>>> a  quick review.
>>
>>
>> Beside minor nitpicks, looks good :-)
> 
> 
> Like? :-P

Ok, like Hashtable usage:

+    this.x_parameters = new Hashtable();

Instead of HashMap (with default map size):

+    this.x_parameters = new HashMap(3);


Or, like doing:

+    this.x_source = atts.getValue(SRC_ATTRIBUTE);
+    if ((this.x_source == null) || (this.x_source.length() == 0)) {
+        throw new SAXException("Unspecified \"src\" attribute");

When you just defined a constant ;-)

+    this.x_source = atts.getValue(SRC_ATTRIBUTE);
+    if ((this.x_source == null) || (this.x_source.length() == 0)) {
+        throw new SAXException("Unspecified \"" + SRC_ATTRIBUTE + "\" 
attribute");

(Same with:
+        throw new SAXException("Unspecified \"name\" attribute");
)


I'd also go with null this.x_parameters by default, and creating HashMap 
  when necessary, and this.x_parameters = null instead of .clear() - 
saves some cycles in most commonly used scenarios (no parameters).

:-P

Vadim


Mime
View raw message