On Sat, Dec 18, 2010 at 1:29 PM, Afkham Azeez <[email protected]> wrote:

> This code has been written very well indeed. Good job.


+1.

>
> I have a few comments as usual :)
>
> See
>
> https://svn.apache.org/repos/asf/synapse/trunk/java/modules/core/src/main/java/org/apache/synapse/util/xpath/Base64EncodeFunction.java
>
>
> <https://svn.apache.org/repos/asf/synapse/trunk/java/modules/core/src/main/java/org/apache/synapse/util/xpath/Base64EncodeFunction.java>Why
> is, NULL_STRING = "". Why do we need a constant for this. It is good to use
> constants, but they should be given meaningful names;
>
> e.g.
> int ONE = 1; // BAD!
> int DEFAULT_QUANTITY = 1; // Good!
>

few more things.

public Object call(Context context, List args) throws FunctionCallException

This function comment says it returns the a string. Then better to put
the return vale as String.

private Object encode(boolean debugOn, String encoding, String value)

I think it is not necessary to pass degugOn as a parameter to a
function. And this also returns a string.
And also function code can be bit optimize by using one try catch block.

thanks,
Amila.





>
> You have also used the *...@override annotation* which is a very good
> practice. It is not mandatory to have that in Java, but when you program
> defensively, you should alway use that. The main advantage of this
> annotation is that if the super class method is removed in the future, you
> will get a compilation error, which will allow you to catch problems early.
>
> Keep up the good work.
>
> Azeez
>
> On Sat, Dec 18, 2010 at 8:22 AM, Supun Kamburugamuva <[email protected]>wrote:
>
>> Recently Udayanga did an improvement to Synapse XPath extensions. Now a
>> user can write their own XPath extensions to synapse.
>>
>> I just wanted to point out that the code was written nicely with comments
>> and the coding style was good. It is nice to see good codes contributed to
>> our projects.
>>
>> The code can be found at [1]
>>
>> [1]
>> https://svn.apache.org/repos/asf/synapse/trunk/java/modules/core/src/main/java/org/apache/synapse/util/xpath
>>
>> Thanks,
>> --
>> Supun Kamburugamuva
>> Technical Lead
>> WSO2 Inc.;  http://wso2.org
>> E-mail: [email protected];  Mobile: +94 77 431 3585
>> Blog: http://supunk.blogspot.com
>>
>> _______________________________________________
>> Carbon-dev mailing list
>> [email protected]
>> https://wso2.org/cgi-bin/mailman/listinfo/carbon-dev
>>
>>
>
>
> --
> *Afkham Azeez*
> Senior Software Architect & Senior Manager; WSO2, Inc.; http://wso2.com,
> *
> *
> *Member; Apache Software Foundation; 
> **http://www.apache.org/*<http://www.apache.org/>
> *
> email: **[email protected]* <[email protected]>* cell: +94 77 3320919
> blog: **http://blog.afkham.org* <http://blog.afkham.org>*
> twitter: **http://twitter.com/afkham_azeez*<http://twitter.com/afkham_azeez>
> *
> linked-in: **http://lk.linkedin.com/in/afkhamazeez*
> *
> *
> *Lean . Enterprise . Middleware*
>
>
> _______________________________________________
> Carbon-dev mailing list
> [email protected]
> https://wso2.org/cgi-bin/mailman/listinfo/carbon-dev
>
>
_______________________________________________
Carbon-dev mailing list
[email protected]
https://wso2.org/cgi-bin/mailman/listinfo/carbon-dev

Reply via email to