Skip to content

Conversation

@cebe
Copy link

@cebe cebe commented Jan 15, 2018

Removed references to properties on $this which is not needed as of
PHP 5.4, which is the minimum requirement in composer.json.

https://secure.php.net/manual/en/functions.anonymous.php

As of PHP 5.4.0, when declared in the context of a class, the current
class is automatically bound to it, making $this available inside of
the function's scope.

Removed references to properties on `$this` which is not needed as of
PHP 5.4, which is the minimum requirement in composer.json.

> https://secure.php.net/manual/en/functions.anonymous.php
>
> As of PHP 5.4.0, when declared in the context of a class, the current
> class is automatically bound to it, making $this available inside of
> the function's scope.
@kelunik
Copy link

kelunik commented Jan 15, 2018

Just a note, not sure whether it applies to this code: Such constructions are sometimes used to avoid cyclic references.

@cebe
Copy link
Author

cebe commented Jan 15, 2018

This code does not avoid any references, it is just a complicated way to express a reference to a class property that can be directly accesses instead. This was necessary with PHP 5.3, but it is not necessary anymore in PHP 5.4, so since a0becdb#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780L7 it can be changed.

Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

@cebe Thank you for your contribution! 👍 I agree that these references aren't needed right now and as such it makes sense to remove these.

Note that I'm currently working on #41 which will eventually remove the complete Request class anyway.

In the meantime, it may make sense to get this suggested PR in 👍

@jsor jsor closed this in #127 Feb 9, 2018
@cebe cebe deleted the cleanup branch February 16, 2018 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants