BUG: TypeConverter<<isRightNullValue: does not use rightNullValue...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

BUG: TypeConverter<<isRightNullValue: does not use rightNullValue...

Christopher J. Demers
TypeConverter<<isRightNullValue: does not use rightNullValue for its test,
it just does an isNil test.  I believe it should test against rightNullValue
just like isLeftNullValue: does with leftNullValue.  The symptom of the bug
can be experienced by creating a TextEdit with a DateToText type converter.
Valid dates are fine, but if you delete the date it still tries to create
one from the empty string, when instead it should just return the null
value.

Chris


Reply | Threaded
Open this post in threaded view
|

Re: TypeConverter<<isRightNullValue: does not use rightNullValue...

Blair McGlashan
"Christopher J. Demers" <[hidden email]> wrote in
message news:b0pqgo$s4heu$[hidden email]...
> TypeConverter<<isRightNullValue: does not use rightNullValue for its test,
> it just does an isNil test.  I believe it should test against
rightNullValue
> just like isLeftNullValue: does with leftNullValue.  The symptom of the
bug
> can be experienced by creating a TextEdit with a DateToText type
converter.
> Valid dates are fine, but if you delete the date it still tries to create
> one from the empty string, when instead it should just return the null
> value.

Hmmm, this rang a bell, and indeed there is an old bug report on our system
(#337: "TypeConverter>>is(Left|Right)ExceptionValue: should compare against
left/right exception value, rather than just test for nil."), but it would
appear that the change (in 5.0) was only partially made. Unfortunately it is
not clear whether that was an oversight, or whether it was deliberate. I'll
reopen it, but I'm not sure it is safe enough to put into PL2, although I am
willing to accept arguments to the contrary.

Regards

Blair


Reply | Threaded
Open this post in threaded view
|

Re: TypeConverter<<isRightNullValue: does not use rightNullValue...

Christopher J. Demers
"Blair McGlashan" <[hidden email]> wrote in message
news:b168db$vhleq$[hidden email]...
> "Christopher J. Demers" <[hidden email]> wrote in
> message news:b0pqgo$s4heu$[hidden email]...
> > TypeConverter<<isRightNullValue: does not use rightNullValue for its
test,
> > it just does an isNil test.  I believe it should test against

> Hmmm, this rang a bell, and indeed there is an old bug report on our
system
> (#337: "TypeConverter>>is(Left|Right)ExceptionValue: should compare
against
> left/right exception value, rather than just test for nil."), but it would
> appear that the change (in 5.0) was only partially made. Unfortunately it
is
> not clear whether that was an oversight, or whether it was deliberate.
I'll
> reopen it, but I'm not sure it is safe enough to put into PL2, although I
am
> willing to accept arguments to the contrary.

I don't know if the "fix" might cause additional problems, but I know the
current state of the code does indeed cause problems.  Try making a date
field where the user is allowed to not enter a date.  If the user clears the
date field an error will result because it will still try to convert it to a
date since an empty string does not = nil.  I made the obvious code change,
and it seems to work.  I am sure more testing is in order, but I do think
this should be resolved at some point.

Chris


Reply | Threaded
Open this post in threaded view
|

Re: TypeConverter<<isRightNullValue: does not use rightNullValue...

Blair McGlashan
"Christopher J. Demers" <[hidden email]> wrote in
message news:b1cjic$11ok2e$[hidden email]...
>...
> I don't know if the "fix" might cause additional problems, but I know the
> current state of the code does indeed cause problems.  Try making a date
> field where the user is allowed to not enter a date.  If the user clears
the
> date field an error will result because it will still try to convert it to
a
> date since an empty string does not = nil.  I made the obvious code
change,
> and it seems to work.  I am sure more testing is in order, but I do think
> this should be resolved at some point.

I appreciate that, but I'm concerned that patching it might possibly break
applications. While we don't shy away from making changes that may not be
backwards compatible between major releases, if we think it necessary, we do
try not to make such changes in a patch release.  Since the patch is near
release we don't feel we ourselves have a sufficient time to
reason/test/gain confidence that the change would be OK. We certainly intend
to change it for the next release, however.

Regards

Blair