search

ucam-department-of-psychiatry / camcops

Cambridge Cognitive and Psychiatric Test Kit (CamCOPS)
Other
12 stars 8 forks source link

StrictDoubleValidator rejecting certain valid values #368

Closed martinburchell closed 3 weeks ago

martinburchell commented 3 months ago

I noticed a couple of problems with StrictDoubleValidator::validate().

When I experimented with changing the minimum height for the BMI task from 0 to 0.1. I was unable to enter 0 on the way to entering a value greater than 0.

I fixed one potential problem in numericfunc.cpp where the floating point to string conversion for the ranges in numDigitsDouble() with the default 50 decimal places was ending up with strings of different lengths. A number like 3 would end up something like 3.0000000000000001234 so stripping the trailing zeros didn't work.

Before doing anything further I added some automated tests for StrictDoubleValidator, one for the problem I was experiencing and one that is a general stress test simulating entering lots of random valid numbers character by character and checking that the validator doesn't reject each step. It fails for the number -124401 within the range -154620 to -113217.

I'm wondering if character by character validation of doubles is actually a really hard problem to solve and also whether a better user experience would be for them to be able to type invalid values but for the field to show up as incomplete until the values are valid. It may be that we can make use of the ValidatingLineEdit widget currently used in the PatientRegistrationDialog, which shows red/green for valid/invalid input and a Valid/Invalid label beneath. There would be some work to refactor the text changed callbacks as both the ValidatingLineEdit widget and QuLineEdit currently handle these.

My experiments are on the bmi-zero-height-fix branch.

Related to #366 though it looks like the better approach here would be to add an opt-out option to the BMI task.

RudolfCardinal commented 2 months ago

Altered (https://github.com/ucam-department-of-psychiatry/camcops/commit/5a138538f9f64e75af781521180d1f83a4e27805 and https://github.com/ucam-department-of-psychiatry/camcops/commit/68d7a8e336abda32606953322a35b620a2843b77) -- I'm not sure it's perfect, in that there is potential for some Invalid values to be marked as Intermediate. But I think it is better!

(I tried a recursive "typing" method but that was just painfully slow.)

RudolfCardinal commented 2 months ago

But agree that the more general problem is opting out of scheduled tasks.

martinburchell commented 3 weeks ago

Fixed by #370