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

Remove double title, make example use sessions, remove .inc usage. #2756

Closed
wants to merge 0 commits into from

Conversation

alfsb
Copy link
Member

@alfsb alfsb commented Sep 11, 2023

The page mentions session_register/$_SESSION, but these dont occurs on example.

Also, the example use obsolete and insecure .inc extension for included files.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Considering you are basically rewriting this whole page already, maybe address the TODO comment at the top of the file?

<!-- TODO Rewrite to remove usage of "you" and talk about __serialize/_unserialize -->


<para>
<function>serialize</function> returns a string containing a
byte-stream representation of any value that can be stored in
PHP. <function>unserialize</function> can use this string to
recreate the original variable values. Using serialize to
save an object will save all variables in an object. The
save an object will save all variables in an object. The
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this white space change. 2 spaces after a FULL STOP is a valid (albeit less used) English formatting style.

Copy link
Member Author

Choose a reason for hiding this comment

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

About this, all UAs effectively replaces any runs of whitespace with a single space:

image

The manual is only served and rendered as HTML, even in download formats, as seen on https://www.php.net/download-docs.php :

Single HTML file Many HTML files HTML Help file HTML Help file (with user notes)

Copy link
Member

Choose a reason for hiding this comment

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

I know this, but I don't see the point in changing this, it is just noise.

Comment on lines 18 to 27
In order to be able to <function>unserialize</function> an object, the
class of that object needs to be defined. That is, if you have an object
of class A and serialize this, you'll
get a string that refers to class A and contains all values of variables
of class <literal>UserClass</literal> and serialize this, you'll
get a string that refers to UserClass and contains all values of variables
contained in it. If you want to be able to unserialize
this in another file, an object of class A, the
definition of class A must be present in that file first.
This can be done for example by storing the class definition of class A
this in another file, an object of UserClass, the
definition of UserClass must be present in that file first.
This can be done for example by storing the class definition of UserClass
in an include file and including this file or making use of the
Copy link
Member

Choose a reason for hiding this comment

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

Add literal tags around all the UserClass maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but it will pollute the text with formating, after UserClass is highlighted once.

Now:

In order to be able to unserialize an object, the class of that object needs to be defined. That is, if you have an object of class UserClass and serialize this, you'll get a string that refers to UserClass and contains all values of variables contained in it. If you want to be able to unserialize this in another file, an object of UserClass, the definition of UserClass must be present in that file first. This can be done for example by storing the class definition of UserClass in an include file and including this file or making use of the spl_autoload_register function.

Changed:

In order to be able to unserialize an object, the class of that object needs to be defined. That is, if you have an object of class UserClass and serialize this, you'll get a string that refers to UserClass and contains all values of variables contained in it. If you want to be able to unserialize this in another file, an object of UserClass, the definition of UserClass must be present in that file first. This can be done for example by storing the class definition of UserClass in an include file and including this file or making use of the spl_autoload_register function.

Copy link
Member

Choose a reason for hiding this comment

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

I don't really mind either way

Copy link
Contributor

Choose a reason for hiding this comment

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

Why only mark up the first UserClass? I prefer either all or none. (What was wrong with class A anyway?)

Comment on lines 31 to 44
<programlisting role="php">
<![CDATA[
// UserClass.php:
<?php
// classa.inc:
class UserClass {
public $data = "";

function __construct($value)
{
$this->data = $value;
}

class A {
public $one = 1;

public function show_one() {
echo $this->one;
}
}
public function show() {
echo $this->data;
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it makes sense to split the different "files" into separate <programlisting> and CDATA tags? And use the title tag to name the file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok for this and others, so will take some time.

@@ -77,9 +83,10 @@

<para>
So if in the example above <varname>$a</varname> became part of a session
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is variable $a?

file <literal>classa.inc</literal> on all of your pages, not only <filename>page1.php</filename>
and <filename>page2.php</filename>.
by adding a new key to the <varname>$_SESSION</varname> superglobal array,
you should include the file <literal>classa.inc</literal> on all of your
Copy link
Contributor

Choose a reason for hiding this comment

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

What is classa.inc?

@@ -2,66 +2,72 @@
<!-- $Revision$ -->
<!-- TODO Rewrite to remove usage of "you" and talk about __serialize/_unserialize -->
<sect1 xml:id="language.oop5.serialization" xmlns="http://docbook.org/ns/docbook">
<title>Object Serialization</title>
<title>Serializing objects - objects in sessions</title>
<title>Object serialization and sessions</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

👎 on this change. The page title should stay as it was. Much of the page is a general overview of serializing objects. There can be a sub-section (<sect2>) for the part about serialized objects in sessions, if you want to separate the details about sessions in particular.

@salathe
Copy link
Contributor

salathe commented Sep 13, 2023

Given the current set of changes, I don't like the only example on the page being about $_SESSION and it's automagic (un)serialization of objects. Can we keep a very basic "my first introduction to serializing objects" example?

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.

3 participants