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

YOURLS Coding Standards Update #2389

Open
PopVeKind opened this Issue Apr 18, 2018 · 2 comments

Comments

Projects
None yet
1 participant
@PopVeKind
Contributor

PopVeKind commented Apr 18, 2018

The YOURLS Coding Standards have not been updated in over four years. I would like to make clear some points that I have had PR's rejected for and add these to the Coding Standards.

No Static Vars

  • Static Vars make testing impossible once a value has been set
  • Static Vars prevent the function to be actually shuntable after the value has been set

See HERE and fc69856

No Singleton

No Singleton (Someclass::someproperty) because it's a unit test nightmare. If we move to something else than actually used, it would be dependency injection.

No Constants

  • Defined Constants are Problematic for testing.
  • Constants are not needed in YOURLS.
  • Defining Constants often results in added Support Issues.
  • Constants are Depreciated in YOURLS.

TIPS

  • Constants may be replaced by Database Options in Plugins.
  • Core Plugins should replace Core Options.

EXCEPTION

  • The Five Constants in config.php which define the database.

No Core Options

  • A core option is usually not necessary and confuses noobs.
  • Core options should be replaced with Core Plugins that set optional values in the database.
  • Very few sites require anything different than the Default Values.
  • It seems unreasonable that all sites would be forced to set 10 or more options when very few will even need to set one!

TIPS

  • Core Options may be replaced by Core Plugins as used as needed.
  • Core Plugins may set Database Options for those few who require it.

$_SERVER Usage

Limit $_SERVER element use to those elements used in the WordPress core.

  • Some coders believe the PHP property $_SERVER is not reliable because they only read the PHP manual and they ignore the Web Server (Apache, nginx, etc.) manual. By reading the manual one finds out $_SERVER is very reliable.
  • Some elements of $_SERVER are populated differently based on the server(Apache, nginx, etc.).
  • YOURLS, as well as WordPress (with 75 million installed sites), depend on elements of $_SERVER
  • It is wise to limit the element use of $_SERVER to those used and tested in the WordPress Core.
@PopVeKind

This comment has been minimized.

Contributor

PopVeKind commented Oct 20, 2018

Do NOT use $_REQUEST

Instead, use any of these (as appropriate):

  • $_GET
  • $_POST
  • $_COOKIE

@PopVeKind

This comment has been minimized.

Contributor

PopVeKind commented Oct 24, 2018

Do NOT use $_SERVER['SERVER_NAME']

Instead use:

  • $_SERVER['HTTP_HOST']

$_SERVER['HTTP_HOST'] gives you the domain name through which the current request is being fulfilled and is more directly related to the request.

HTTP_HOST is typically more useful in most applications in that it relates directly to the request, whereas SERVER_NAME could return whatever value is in the conf file and doesn't tell you anything about the request at all. SERVER_NAME also tends to fail behind some proxies, load-balancers and reverse-proxies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment