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



Reply via email to