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 improper uses of references #317

Open
hemberger opened this issue Jan 24, 2018 · 0 comments
Open

Remove improper uses of references #317

hemberger opened this issue Jan 24, 2018 · 0 comments
Labels

Comments

@hemberger
Copy link
Member

hemberger commented Jan 24, 2018

In strict PHP, you can only assign variables by reference. Things
that are not variables include function calls and constants.

So we begin the herculean task of removing all unneeded references.
Here are some important details about references:

  • Objects are always passed/copied by reference (in the c++ sense).
    We can pass an object without the & and still access/modify
    its internal data.

  • Arrays are passed/copied by value, but not really because of
    copy-on-write, which means that it will only make a copy of
    the array if you try to modify it.

  • References disable copy-on-write. If you have an array that was
    copied by value, and then you take a reference to it, you will
    immediately make a copy of it, even if the reference doesn't
    modify that array.

Moral of the story

Do not use references in an attempt to improve performance. PHP
does all the right performance things behind the scenes if
you are using the language in the intended way.

References are only intended for when you truly want to pass
something to another function and have that function modify it.

For more details, see
http://schlueters.de/blog/archives/125-Do-not-use-PHP-references.html

hemberger added a commit to hemberger/smr that referenced this issue Feb 21, 2018
See smrealms#317.

In strict PHP, you can only assign variables by reference. Things
that are not variables include function calls and constants.

So we begin the herculean task of removing all unneeded references.
Here are some important details about references:

* Objects are always passed/copied by reference (in the c++ sense).
  We can pass an object without the `&` and still access/modify
  its internal data.

* Arrays are passed/copied by value, but not really because of
  copy-on-write, which means that it will only make a copy of
  the array if you try to modify it.

* References disable copy-on-write. If you have an array that was
  copied by value, and then you take a reference to it, you will
  immediately make a copy of it, even if the reference doesn't
  modify that array.

>>> Moral of the story <<<

Do not use references in an attempt to improve performance. PHP
does all the right performance things behind the scenes _if_
you are using the language in the intended way.

References are only intended for when you truly want to pass
something to another function and have that function modify it.

For more details, see
http://schlueters.de/blog/archives/125-Do-not-use-PHP-references.html
hemberger added a commit to hemberger/smr that referenced this issue Feb 21, 2018
See smrealms#317.

Fixes some warnings in PHP 7+ about only variables being passed
by reference (by removing the references).

Note: this does not change any of the interfaces; they will be
modified at a later date.
hemberger added a commit to hemberger/smr that referenced this issue Feb 23, 2018
See smrealms#317.

In strict PHP, you can only assign variables by reference. Things
that are not variables include function calls and constants.

So we begin the herculean task of removing all unneeded references.
Here are some important details about references:

* Objects are always passed/copied by reference (in the c++ sense).
  We can pass an object without the `&` and still access/modify
  its internal data.

* Arrays are passed/copied by value, but not really because of
  copy-on-write, which means that it will only make a copy of
  the array if you try to modify it.

* References disable copy-on-write. If you have an array that was
  copied by value, and then you take a reference to it, you will
  immediately make a copy of it, even if the reference doesn't
  modify that array.

>>> Moral of the story <<<

Do not use references in an attempt to improve performance. PHP
does all the right performance things behind the scenes _if_
you are using the language in the intended way.

References are only intended for when you truly want to pass
something to another function and have that function modify it.

For more details, see
http://schlueters.de/blog/archives/125-Do-not-use-PHP-references.html
hemberger added a commit to hemberger/smr that referenced this issue Feb 23, 2018
See smrealms#317.

Fixes some warnings in PHP 7+ about only variables being passed
by reference (by removing the references).

Note: this does not change any of the interfaces; they will be
modified at a later date.
hemberger added a commit to hemberger/smr that referenced this issue Feb 23, 2018
hemberger added a commit to hemberger/smr that referenced this issue Feb 23, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 7, 2018
Fix "Only variables should be assigned by reference" warnings.

Also remove a bunch of unnecessary uses of references. See smrealms#317.
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Mar 27, 2018
hemberger added a commit to hemberger/smr that referenced this issue Apr 1, 2018
Arguments should only be passed by reference into a function if
the function intends to modify the original variable. It should
not be used to try to improve performance (it won't: see smrealms#317).

Here we remove improper references from class method arguments,
along with a few other internal references that I missed earlier.
hemberger added a commit to hemberger/smr that referenced this issue Apr 2, 2018
Arguments should only be passed by reference into a function if
the function intends to modify the original variable. It should
not be used to try to improve performance (it won't: see smrealms#317).

Here we remove improper references from class method arguments,
along with a few other internal references that I missed earlier.
hemberger added a commit to hemberger/smr that referenced this issue Apr 3, 2018
See smrealms#317.

Also fix spelling of `doSkeletonAssigns`.
hemberger added a commit to hemberger/smr that referenced this issue Apr 3, 2018
See smrealms#317.

We should never need to modify a variable that was assigned to
a template (because templates are for display only, not processing).
Therefore, we should only pass variables by reference to the template
to improve both performance and safety.

* `assign` and `display` no longer use references
* `assignByRef` has been removed
hemberger added a commit to hemberger/smr that referenced this issue Apr 21, 2018
hemberger added a commit to hemberger/smr that referenced this issue Apr 30, 2018
See smrealms#317.

NOTE: objects used as globals must still be assigned using references.
Doing a standard assignment to `$sector`, for example, results in the
green sector showing up as the current sector on the Local Map!
hemberger added a commit to hemberger/smr that referenced this issue May 8, 2018
hemberger added a commit to hemberger/smr that referenced this issue May 16, 2018
The argument to this function should not be passed by reference.

See smrealms#317.
hemberger added a commit to hemberger/smr that referenced this issue May 16, 2018
Modify all places that call `getPlottedCourse` to return by value.

See smrealms#317.
hemberger added a commit to hemberger/smr that referenced this issue Jun 7, 2018
hemberger added a commit to hemberger/smr that referenced this issue Jun 17, 2018
See smrealms#317.

Remove unnecessary references in variable assignment.
I tried to ensure that every change was tested.
hemberger added a commit to hemberger/smr that referenced this issue Dec 7, 2018
* No more return by reference allowed. See smrealms#317.

* Remove getAccountByHofName (unused).

* Add early (null) return if input is empty. This avoids an extra
  DB query, but also is an extra sanity check because none of these
  account fields should EVER be empty. And we want to be very
  careful not to accidentally give players access to accounts they
  do not own.
hemberger added a commit that referenced this issue Dec 25, 2018
* No more return by reference allowed. See #317.

* Remove getAccountByHofName (unused).

* Add early (null) return if input is empty. This avoids an extra
  DB query, but also is an extra sanity check because none of these
  account fields should EVER be empty. And we want to be very
  careful not to accidentally give players access to accounts they
  do not own.
hemberger added a commit to hemberger/smr that referenced this issue Mar 8, 2019
This commit removes many references returned by class methods.

See smrealms#317.
hemberger added a commit to hemberger/smr that referenced this issue Mar 11, 2019
This commit removes many references returned by class methods.

See smrealms#317.
hemberger added a commit to hemberger/smr that referenced this issue Mar 11, 2019
Now that most variables are assigned by value instead of by reference,
we can modify our function interfaces to only return results by value.

See smrealms#317.
hemberger added a commit to hemberger/smr that referenced this issue Mar 15, 2019
See smrealms#317.

This handles three major remaining usages:

* When returning objects or arrays from class methods.
  Objects should almost always be returned "by value" (since they
  are handled internally as references by PHP). Arrays should be
  returned "by value" except, perhaps, if they will be modified.

* In foreach loops. The only time a reference is needed here is
  when you want to loop over an array and modify array elements.

* In template includes. We never want to modify variables within
  templates, so there is no reason to use references here.
hemberger added a commit to hemberger/smr that referenced this issue Jun 2, 2019
Objects are always "by reference", even without `&`.

See smrealms#317.
hemberger added a commit to hemberger/smr that referenced this issue Oct 16, 2019
* Split classes up into their own files for readability and to permit
  class autoloading.

* Move route-related classes into "Routes" namespace for organizational
  purposes.

* Remove improper use of references. See smrealms#317.

* Add typehints to function interfaces.
hemberger added a commit to hemberger/smr that referenced this issue Dec 26, 2019
* Split classes up into their own files for readability and to permit
  class autoloading.

* Move route-related classes into "Routes" namespace for organizational
  purposes.

* Remove improper use of references. See smrealms#317.

* Add typehints to class method interfaces and class attributes.
hemberger added a commit to hemberger/smr that referenced this issue Jan 2, 2020
See smrealms#317.

We have incrementally removed most usages of functions returning by
reference; therefore, we can now remove the ability to return by
reference for most functions.
hemberger added a commit to hemberger/smr that referenced this issue Jan 2, 2020
See smrealms#317.

We have incrementally removed most usages of functions returning by
reference; therefore, we can now remove the ability to return by
reference for most functions.
hemberger added a commit to hemberger/smr that referenced this issue Jan 2, 2020
See smrealms#317.

We have incrementally removed most usages of functions returning by
reference; therefore, we can now remove the ability to return by
reference for most functions.
hemberger added a commit that referenced this issue May 26, 2020
See #317.

In these cases, `$board` is an array that is not modified, and `$p`
is a ChessPiece instance.
hemberger added a commit that referenced this issue May 26, 2020
See #317.

Here, `$moves` is an array that is not modified, and `$p` is a
ChessPiece instance.
hemberger added a commit to hemberger/smr that referenced this issue Mar 1, 2021
See smrealms#317.

The reference assignments in the templates were especially dangerous
because undefined variables are created if they are referenced, e.g.

$a = $b;  // Undefined variable warning
$a = &$b; // No warning, $a and $b are set to null

We had some conditionally defined combat result array keys being used,
so now we check for existence first. Some copy+paste errors were also
fixed.
hemberger added a commit that referenced this issue Mar 28, 2021
hemberger added a commit to hemberger/smr that referenced this issue Apr 25, 2021
hemberger added a commit to hemberger/smr that referenced this issue Apr 25, 2021
hemberger added a commit to hemberger/smr that referenced this issue Apr 26, 2021
hemberger added a commit to hemberger/smr that referenced this issue Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant