toString(null) -> "null" or "" [was: Valid filter?]

[Felix Knecht <fe...@apache.org>]

Imagine following test:

String str="(objectClass=)";
ExprNode node = FilterParser.parse( str );
assertEquals( str, node.toString() );

This test will fail ATM because we do
'return value == null ? "null" : value;'

This is done in
- org.apache.directory.shared.ldap.entry.StringValue
- org.apache.directory.shared.ldap.entry.BinaryValue

So what's closer to what we really want to show?

My +1 for returning an empty String instead if "null"
'return value == null ? "" : value;'


Felix

Re: toString(null) -> "null" or "" [was: Valid filter?]

[Emmanuel Lecharny <el...@gmail.com>]

On 10/29/10 7:57 PM, Felix Knecht wrote:
>
>> Note that it won't be enough. We have to do the same thing in other
>> XXXNode classes.
>
> Thanks for hints. Doing so it makes the null-string question obsolete.
> Hope I got all the XXXNode classes (found 4)

Thanks a bunch Felix !

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com

Re: toString(null) -> "null" or "" [was: Valid filter?]

[Felix Knecht <fe...@apache.org>]

> Note that it won't be enough. We have to do the same thing in other
> XXXNode classes.

Thanks for hints. Doing so it makes the null-string question obsolete.
Hope I got all the XXXNode classes (found 4)

Felix

Re: toString(null) -> "null" or "" [was: Valid filter?]

[Emmanuel Lecharny <el...@gmail.com>]

On 10/29/10 5:31 PM, Felix Knecht wrote:
> Imagine following test:
>
> String str="(objectClass=)";
> ExprNode node = FilterParser.parse( str );
> assertEquals( str, node.toString() );
>
> This test will fail ATM because we do
> 'return value == null ? "null" : value;'

In fact, the node.toString() is throwing a NPE. There are two problems 
here :
- first we don't check that the wrapped value is null before returning 
an escapedValue when we try to create one => NPE
- second, if we do, as the Value will be null, the toString() applied on 
this value will return "null".
>
> So what's closer to what we really want to show?
We should fix the filter toString() method plus the AbstractExprNode 
escapeFilterValue() method :

     protected static Value<?> escapeFilterValue( Value<?> value )
     {
         if ( value.isNull() )
         {
             return value;
         }
...

and in EqualityNode :

     public String toString()
     {
         StringBuilder buf = new StringBuilder();

         buf.append( '(' );

...
         Value<?> escapedValue = getEscapedValue();

         buf.append( "=" );

         if ( !escapedValue.isNull() )
         {
             buf.append( escapedValue );
         }

Note that it won't be enough. We have to do the same thing in other 
XXXNode classes.


-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com

Re: toString(null) -> "null" or "" [was: Valid filter?]

[Kiran Ayyagari <ka...@apache.org>]

On 10/29/10, Felix Knecht <fe...@apache.org> wrote:
> Imagine following test:
>
> String str="(objectClass=)";
> ExprNode node = FilterParser.parse( str );
> assertEquals( str, node.toString() );
>
> This test will fail ATM because we do
> 'return value == null ? "null" : value;'
>
> This is done in
> - org.apache.directory.shared.ldap.entry.StringValue
> - org.apache.directory.shared.ldap.entry.BinaryValue
>
> So what's closer to what we really want to show?
>
> My +1 for returning an empty String instead if "null"
> 'return value == null ? "" : value;'
+1
>
>
> Felix
>


-- 
Kiran Ayyagari