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

Implement support for var_export(new Map()) allowing full class reconstruction #21

Merged
merged 9 commits into from
Sep 25, 2015

Conversation

mglinski
Copy link
Contributor

Implement New NullLogger class to support __set_state() by default

*
* This logger extends the PSR Compliant NullLogger to implement __set_state()
* This allows default support for var_export() compatable code generation.
* Any logger you implement will n
Copy link
Contributor

Choose a reason for hiding this comment

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

@mglinski Incomplete doc?

* @return string Implementaiton of logger class to be passed to the Map class
*/
public static function __set_state($mapData) {
$map = new Map($mapData['map'], $mapData['logger']);
Copy link
Contributor

Choose a reason for hiding this comment

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

@mglinski This should be changed to:

public static function __set_state(array $mapData) {
    return new static($mapData['map'], $mapData['logger']);
}

Otherwise extended classes will be converted to the parent class: http://codepad.viper-7.com/9dmDCy

@mglinski
Copy link
Contributor Author

Fixed.

* @return string Implementaiton of logger class to be passed to the Map class
*/
public static function __set_state($objData = array()) {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

@mglinski This can't return null, it must return a valid \Psr\Log\LoggerInterface or the Map class will break. In fact, you should add a test for \Zumba\Swivel\Map that calls \Zumba\Swivel\Map::enabled after the object has been recreated from a var_export (enabled calls the logger).

Copy link
Contributor

Choose a reason for hiding this comment

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

@mglinski correction: the class won't break. But it should still return a NullLogger so that swivel doesn't have to create another one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, fixing it now.

…d always return an instance of the parent class, never a scalar/null.
young-steveo added a commit that referenced this pull request Sep 25, 2015
Implement support for var_export(new Map()) allowing full class reconstruction
@young-steveo young-steveo merged commit 6936e7a into master Sep 25, 2015
@young-steveo young-steveo deleted the var_export_support branch September 25, 2015 18:25
@mglinski mglinski mentioned this pull request Sep 25, 2015
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

Successfully merging this pull request may close these issues.

2 participants