php-rql icon indicating copy to clipboard operation
php-rql copied to clipboard

testing responce

Open mbrevda opened this issue 8 years ago • 8 comments

It would seem that testing a response is extremely cumbersome. First, the response is returned as an object, and needs to be typeset to an array (as all it's properties are private). Next, the individual value needs to be typeset from a float to an int, an inherently "dangerous" operation. Finally, the value can be tested.

In code;

if ((int)(array)$res['errors'] !== 0) {
    return $res;
}

The more intuitive way of testing that I would expect would be:

if ($res->errors !== 0) {
    return $res;
}

Am I missing something, or is there really no better way to do this?

mbrevda avatar Feb 29 '16 07:02 mbrevda

It's actually an ArrayObject, so you can use it just like an array without any casting.

This should work:

if ($res['errors'] !== 0) {
    return $res;
}

danielmewes avatar Feb 29 '16 08:02 danielmewes

We could slightly improve this and set the ARRAY_AS_PROPS flag (http://php.net/manual/en/class.arrayobject.php#arrayobject.constants.array-as-props). That would make your example $res->errors work.

I'm not sure how this behaves for array indexes that are not valid PHP field names though (for example PHP keywords). We would need to test that.

danielmewes avatar Feb 29 '16 08:02 danielmewes

Hi Daniel! Thanks!

Should it be the same for table creation? The fails every time:

<?php

use r\Queries\Dbs\Db as rDb;
use r\Queries\Tables\TableCreate;

$conn = $di->get('r\Connection');
$rdb = new rDb($conn->defaultDbName);
$res = $conn->run((new TableCreate($rdb, 'teams')));

if ($res['tables_created'] !== 1) {
    throw new Exception('Table "teams" not created');
    exit(1);
}

mbrevda avatar Feb 29 '16 08:02 mbrevda

Oh sorry I forgot that it's still a float. So you'll still need to cast to (int), or use != comparison instead of !==.

danielmewes avatar Feb 29 '16 08:02 danielmewes

The ReQL type system doesn't distinguish between floats and integers unfortunately. I'm not sure if there's anything we can do to make this easier.

danielmewes avatar Feb 29 '16 08:02 danielmewes

!= has the issue linked to above. Is there any way to have an int returned from the system? If not, I would recommend having the drive typehint the values, as to signal to the user 'this is a safe int'.

mbrevda avatar Feb 29 '16 08:02 mbrevda

I think using != is fine in these cases. The values are in a range where the floating point representation is exact. Or am I missing a part of the problem?

I'm not familiar with type hints outside of function declarations. Do you have a link to some documentation or an example? Would adding typehints to solve this mean that we'd need to special-case certain result fields for certain commands?

danielmewes avatar Feb 29 '16 08:02 danielmewes

The values are in a range where the floating point representation is exact.

I'm no expert, I'm simply quoting the manual: "never trust floating number results to the last digit, and do not compare floating point numbers directly for equality."

I'm not familiar with type hints outside of function declarations. Typehinting is for function arguments. Typecasting converts a values type without needing to pass it to a toSomeType() method (as with strongly typed languages). Sorry if I confused the two. http://php.net/manual/en/language.types.type-juggling.php

Would adding typehints to solve this mean that we'd need to special-case certain result fields for certain commands? Yes, as we don't to mangel ALL values, only those that we are certain should be integers.

It would probably be simpler if there was some way this can be done at the engine level, and only return integers.

On Mon, Feb 29, 2016, 10:20 AM Daniel Mewes [email protected] wrote:

I think using != is fine in these cases. The values are in a range where the floating point representation is exact. Or am I missing part of the problem?

I'm not familiar with type hints outside of function declarations. Do you have a link to some documentation or an example? Would adding typehints to solve this mean that we'd need to special-case certain result fields for certain commands?

— Reply to this email directly or view it on GitHub https://github.com/danielmewes/php-rql/issues/121#issuecomment-190090538 .

mbrevda avatar Feb 29 '16 17:02 mbrevda