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

Fixed: ?? didn't work here as bool won't be seen as null #11673

Merged
merged 4 commits into from Jun 22, 2022

Conversation

Quix0r
Copy link

@Quix0r Quix0r commented Jun 22, 2022

?? didn't catch false here, needs to check explicitly because the returned type is always array.

PS: I just checked all occurrences. And some needed fixing.

@@ -111,7 +111,7 @@ private static function getOldAttachmentData($body)

$picturedata = Images::getInfoFromURLCached($matches[1]);

if ($picturedata) {
if (!empty($picturedata)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty array gets casted as false in a condition, what is the purpose of empty() here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I normally don't like this "magic casting" thing and try to make it clean (with no casting). Plus also in other occurrences I found the empty($array) calls. Should I then rewrite all to one way?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not wrong, just superfluous regarding the basic capabilities of the language. I'd say leave existing instances alone but don't add any more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to make each handling of returned variables from Imagess::getInfoFromURL[Cached]() the same way, that we know this is everywhere the same and don't have to check each case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have now removed empty() from each check to make it be the same on all such occurrences.

@MrPetovan MrPetovan added this to the 2022.09 milestone Jun 22, 2022
@MrPetovan MrPetovan added the Bug label Jun 22, 2022
- empty() is maybe superflous here, still I would prefer a code style that is
  written explicitly and not rely on "magic casting"
@MrPetovan MrPetovan merged commit c824ba4 into friendica:develop Jun 22, 2022
@Quix0r Quix0r deleted the bugs/returned-type-array-bool branch June 22, 2022 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants