Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing responce #121

Open
mbrevda opened this issue Feb 29, 2016 · 8 comments
Open

testing responce #121

mbrevda opened this issue Feb 29, 2016 · 8 comments

Comments

@mbrevda
Copy link
Contributor

mbrevda commented Feb 29, 2016

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?

@danielmewes
Copy link
Owner

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
Copy link
Owner

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.

@mbrevda
Copy link
Contributor Author

mbrevda commented Feb 29, 2016

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);
}

@danielmewes
Copy link
Owner

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
Copy link
Owner

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.

@mbrevda
Copy link
Contributor Author

mbrevda commented Feb 29, 2016

!= 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'.

@danielmewes
Copy link
Owner

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?

@mbrevda
Copy link
Contributor Author

mbrevda commented Feb 29, 2016

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 notifications@github.com
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
#121 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants