Potential logic issue in FeatureValuePathImpl?

[Marshall Schor <ms...@schor.com>]

 In uimaj-core, FeatureValuePathImpl, line 939:

        if (this.childPath != null) {
            // for simple range types, only [] and fsId() are allowed as child
            // path
            if (this.isSimpleRangeType
                    & !(this.childPath.isBracketsOnly() ||
this.childPath.isFsIdFeature)) {
                CASRuntimeException exception = new CASRuntimeException(
                        CASRuntimeException.INVALID_FEATURE_PATH, new String[] {
this.featureName });
                throw exception;
            }

Maybe there are 2 issues:  One (findbugs noted) the second if statement has a
"&" instead of an "&&" ? but I'm thinking there's a negation missing from the
right-side clause of the "||" - if it supposed to correspond to the comment ?

Can anyone confirm this?  I can fix, but I don't know this area very well, so
didn't want to break something :-) .

-Marshall

Re: Potential logic issue in FeatureValuePathImpl?

[Marshall Schor <ms...@schor.com>]

 oops, of course, you're correct.  My mind must have tricked me when I read that
initially, missing the outer parenthesis...  OK - I'll do the fix.

-Marshall

On 8/8/2010 3:06 PM, Adam Lally wrote:
> I don't think there's a missing negation; the entire disjunction is negated
> already.  If neither isBracketsOnly() nor isFsIdFeature is true, it will
> throw an exception.
>
> +1 for changing & to &&.
>
>   -Adam
>
> On Sun, Aug 8, 2010 at 10:23 AM, Marshall Schor <ms...@schor.com> wrote:
>
>>  In uimaj-core, FeatureValuePathImpl, line 939:
>>
>>        if (this.childPath != null) {
>>            // for simple range types, only [] and fsId() are allowed as
>> child
>>            // path
>>            if (this.isSimpleRangeType
>>                    & !(this.childPath.isBracketsOnly() ||
>> this.childPath.isFsIdFeature)) {
>>                CASRuntimeException exception = new CASRuntimeException(
>>                        CASRuntimeException.INVALID_FEATURE_PATH, new
>> String[] {
>> this.featureName });
>>                throw exception;
>>            }
>>
>> Maybe there are 2 issues:  One (findbugs noted) the second if statement has
>> a
>> "&" instead of an "&&" ? but I'm thinking there's a negation missing from
>> the
>> right-side clause of the "||" - if it supposed to correspond to the comment
>> ?
>>
>> Can anyone confirm this?  I can fix, but I don't know this area very well,
>> so
>> didn't want to break something :-) .
>>
>> -Marshall
>>

Re: Potential logic issue in FeatureValuePathImpl?

[Adam Lally <al...@alum.rpi.edu>]

I don't think there's a missing negation; the entire disjunction is negated
already.  If neither isBracketsOnly() nor isFsIdFeature is true, it will
throw an exception.

+1 for changing & to &&.

  -Adam

On Sun, Aug 8, 2010 at 10:23 AM, Marshall Schor <ms...@schor.com> wrote:

>  In uimaj-core, FeatureValuePathImpl, line 939:
>
>        if (this.childPath != null) {
>            // for simple range types, only [] and fsId() are allowed as
> child
>            // path
>            if (this.isSimpleRangeType
>                    & !(this.childPath.isBracketsOnly() ||
> this.childPath.isFsIdFeature)) {
>                CASRuntimeException exception = new CASRuntimeException(
>                        CASRuntimeException.INVALID_FEATURE_PATH, new
> String[] {
> this.featureName });
>                throw exception;
>            }
>
> Maybe there are 2 issues:  One (findbugs noted) the second if statement has
> a
> "&" instead of an "&&" ? but I'm thinking there's a negation missing from
> the
> right-side clause of the "||" - if it supposed to correspond to the comment
> ?
>
> Can anyone confirm this?  I can fix, but I don't know this area very well,
> so
> didn't want to break something :-) .
>
> -Marshall
>