It's time for another good idea, bad idea discussion -- import and the function layout for PHP 7

include, require, require_once, include_once. Do we need another file loader? No. But I wonder if another one might be useful if it is aimed towards how modern PHP code is used - import.

Also, let’s attack the function mess - please - haystack, needle / needle, haystack needs to go.

<?php

import( 'library or /path/to/php/class/file', 'namespace\path\to\import\into');

?>

There are critical differences from include and require

  1. Import expects the whole file to be php code. I know this will play some Hell with IDE’s maybe, but it should make for faster parsing to be able to call a subroutine that doesn’t need to take html token switching into account. I could be wrong.

  2. Import pulls in libraries or class files. It compiles but does not execute them - runtime executable code will be discarded by import, adding a measure of security since a class or function library must both be loaded and called to be executed, where include and require can be tricked into executing code by simply pointing them at a bogus file. To illustrate, consider this file

    class May {
      function moo {
        echo 'moo';
      }
    }
    
    echo 'bad code';

With my proposal, the old include statements wouldn’t execute anything in this poisoned file under PHP 7 because there are no <?php ?> markers. True, the hacker can put them there, but import will parse error if it sees them. And when import pulls the file in, it loads the class but ignores the code outside of it (though perhaps throwing a E_WARNING).

Import also allows for the following change to the function table in PHP. There will be 2 core namespaces, legacy and standard. Standard will be a clean slate, functions with consistent names and argument orders, but also very small - just the most frequently used functions. Most functions will be off in libraries. So if you’re working with MySQL your program would start with

import('mysql');

If for some crazy reason you need to use two database engines at the same time you’d have a problem, but here import saves you by letting you say what namespace the library goes into.

import ('mysql', 'MySQL');
import ('postgre', 'PostGre');

As to the legacy namespace, it would be imported by default to the root namespace by a new PHP.ini setting that would be true in 7 and false in 8 because of the BC breaking havoc turning it off causes. The standard library would only include those functions that don’t namespace collide with legacy when in legacy mode. In standard mode the standard library would load along with some of the more common libraries likely to have collisions, such as ‘String’ and ‘Array’

As for file requires, import can reassign their namespace or append it, depending on how the namespace was setup. If the most common method was used

namespace MyNameSpace;

Then import’s directive is an override. If its done like this

namespace MyFirstNameSpace {

}
namespace MySecondNameSpace {

}

Then these namespaces become children.

So what about the use statement? It works as it always has, so if you import something into a namespace it doesn’t belong in use statements elsewhere would fail. However, this allows for “monkey” typing to be done at the language level, not at the DI level - let me explain.

Right now the new operator creates a hard dependency, we can’t do anything about.

class A {
  public function __construct() {
    $this->property = new B();
  }
}

And we’re stuck. If we need to trade out B for a specific version we have to change A’s code - a problem if A is from someone else’s library. But with import we don’t need to touch A. Here’s how…

import ('/filepath/to/B', 'Original');
import('/filepath/to/new/B');

The class file for new B knows about this dastardly plan.

class B extends Original\B {

}

But class A knows nothing about the fact it’s now dealing with a new B.

Now, is this a stroke of genius or a blast of stupidity? I don’t know.

Totally agree about the haystack and needle, also to standardise on underscore usage.

Two features I would like see:

  1. to be able to compile the source. Compilers are usually excellent at finding errors and warnings.

  2. adopt the Golang approach of having a standard formatting scheme.

edit:
Just remembered that one of our users is active on the PHP documentation. Can anyone remember his name?

@salathe?

1 Like

I think the “import class into namespace” ship has sailed. I actually posted a function on here using eval to do exactly that when PHP5.3 came out. At this point, now libraries are being released with namespaces included then it’s redundant. For PHP7 especially it’s redundant.

This is a good example of the problems of the static new keyword. The solution you provide does solve it, but really the answer here is DI as it’s less magic.

class A {
  public function __construct(B $b) {
    $this->property = $b;
  }
}

The problem with your approach is it moves the problem. If I want to use this behaviour (replacing B by loading it from a different file), I need to know the inner workings of the library.

I’m hoping that the way around all these inconsistencies is scalar objects. $str->replace(‘a’, ‘b’); This would give the developers an opportunity to fix the inconsistencies without breaking BC. $array->combine(), etc…

@TomB

ArrayObject is a start … (but nowhere near a replacement for arrays yet)

on the other-hand-side, if we have scalar objects, we could as well use JavaScript …

There have been several considerations of making available “PHP-code-only” files, and import() takes that a step further with the idea of throwing away non-definition code.

It looks to me like you’re mashing a pile of different concepts together under the “import” heading. Concepts which would probably be better suited to being kept as separate, distinct proposals: loading code into a namespace, reading only definitions from a file, PHP-only files, standard library modularisation, standard library API replacing, monkey patching, etc.

Each have merit for discussion, and obvious problems, and I’m not sure uniting them all under the “import” heading is the way to go.

The needle/haystack issue itself is pretty minor. For string functions that have haystack and needle arguments, haystack comes first. For array functions, needle comes first.

However, if you consider similar functions that conceptually use a “needle” and “haystack”, if not by name, then sure there are some inconsistencies.

I think that people tend to conflate “the needle/haystack problem” with the much more widespread “consistency of function naming, argument naming and ordering problem”. The latter is really coming into the fore as PHP looks to be trying to move away from the idea of being “just” a glue language between C libraries and HTTP output.

1 Like

For what it’s worth it was mostly a musing post. I get to learn a lot about what I don’t know by the answers I get, and none of the points raised I disagree with - particularly Tom’s. I did found out that if you don’t have the ability to write code it’s hard to convince anyone to do something. That, and if I could have one thing in PHP 7 it wouldn’t be one of these. It would changing assert over to a statement instead of a function, with the special behavior that whatever appears between its argument parenthesis will be ignored when assertions are turned off. This currently isn’t the case - if you give use a function that returns true or false with assert it will be called unless you transform it to a string. It’s annoying to read to be honest.

FYI if you did want to play around with this, writing the import function is fairly trivial:

function import($file, $namespace) {
	eval('namespace ' . $namespace . '; ?>' . file_get_contents($file));
}

I think that’s a really good suggestion. That said, I’ve become increasingly less in favour of assertions and design by contract in general as imho it mixes responsibilities. Ever since I started using TDD I found that pre/postconditions in the method essentially mixes the implementation with the test case.

Admittedly it does have some advantages over TDD as you can test more than the I/O and you can use it as part of an integration test as well as unit test… still I’ve found, for me at least, TDD has superseded DBC.

Even when I played with Eiffel I found DBC a little cumbersome.

Assertions are also documentation of programmer expectations which are invaluable to maintenance programmers who may not know the system thoroughly. I’ve been working on bringing them into Drupal in order to help developers avoid some of the more cryptic errors that can happen when you misconfigure things.

Also, in my opinion assertions complement TDD, though when used together their role is reduced to keeping an eye out for problems between internal API’s (which, once the system is completed should never fail). In a large project such as drupal they become even more valuable since you can’t be exactly sure what the third-party programmer is up to, and you can use them to try to stop a disastrous mistake.

One example of this is PHP doesn’t currently have a return typehint ability nor can it typehint scalars. Asserting solves both problems. TDD can insure the integrity of the unit, but assertions are better suited to watching the integrity of the system as a whole.

1 Like

Really good points, I’m going to rethink my use of them and give them a go in my next project. Maybe syntax wise it would be better to make a custom assert function that took a closure?

Can you share a couple of examples of where you found them particularly useful?

Well the example that got me off to my coding adventure in Drupal 8 with the Fault System development comes out of their TwigExtension. When I misconfigured a module I got more than a few cryptic errors that I eventually tracked down to here.

<?php
  public function getLink($text, $url) {
    assert($this->linkGenerator instanceof LinkGeneratorInterface, 'missingGenerator');
    if (!$url instanceof Url) {
      $url = Url::fromUri($url);
    }
    return $this->linkGenerator->generate($text, $url);
  }
?>

I found out later that the designers never expected someone to extend Drupal’s TwigExtension, but rather extend the Symfony TwigExtension. The Dependency Injector is dependent on the config being right to attach the url and link generators and if that config is wrong the class gets started up without those components. The assertion catches this situation.

Test driven design is invaluable, but bad config elements by the 3rd party isn’t one of the things they catch especially well. That said, one is not a replacement for the other - the assertions need to be tested to insure they throw when required to as seen in this method from the unit for the class the above code comes from

<?php
public function testAssertions() {
    $twigExt = new TwigExtensionWrapper();
    $twigExt->unprotectedSetGenerator(new FalseUrlGenerator());
    $twigExt->unprotectedSetLinkGenerator(new FalseLinkGenerator());
    $this->assertAssertionNotThrown();

    $twigExt->getPath('blah');
    $this->assertAssertionsThrown('missingGenerators');

    $twigExt->getUrl('blah');
    $this->assertAssertionsThrown('missingGenerators');

    $twigExt->getUrlFromPath('blah');
    $this->assertAssertionsThrown('missingGenerators');

  }
?>

The TwigExtensionWrapper is a child class that allows invalid Generators to be attached - The actual class stops this with type hinting… The FalseLinkGenerator has the functions of the interface being tested, but doesn’t declare that it’s implementing. This creates a testable situation where the assertion will throw, but the test won’t crash when it moves beyond the assertion and calls the function. This also illustrates the situation that fully testing an assertion can be difficult because by their nature they are checking conditions that should be impossible - so in this case we need not one but two fake out classes to create the situation the assertion is looking for. This goes beyond the situation that inspired the assertions placement - in that event the member was null.

Shouldn’t this have been like a core goal of the language about 10 years ago?

You don’t need to answer. :smile: :wink:

If this is now a core goal of PHP7, I say double fantastic! We really absolutely need to very much do as much as possible to relieve PHP of any surfaces of attack, where devs can point to its idiosyncrasies and say, “it is a poor language”. “It’s just a glue language” being one of those surfaces. Although I think PHP is more than just a glue language, the issue with consistency is most definitely there and needs serious attention and I say, break BC, if you have to, to do it! PHP 7 needs to be a huge improvement over the lack of forward progression on the consistency front, ignored for so many years.

Sorry, if that was a bit off topic, but I needed to get it off my chest. :smile:

Scott

Interesting but I’m not sure about your example, it seems more like an encapsulation problem to me… if the class looked like this (assuming PHP7 type hinting)

	public function __construct(LinkGeneratorInterface $linkGenerator) {
		$this->linkGenerator = $linkGenerator;
	}

	public function getLink($text, string $url) {
                //I've also removed the URL conversion here... as it's technically a LoD violation.
		return $this->linkGenerator->generate($text, $url);
	}

Then the contract is enforced by the argument types and assert becomes irrelevant. This seems more like a band-aid solution to an initial sloppy design than an argument in itself for the use of an assertion here. I can see why you are doing it this way, but it doesn’t really demonstrate a need for assertions. By asking for the LinkGenerator in the constructor I can be sure that it’s correctly set in any other method in the class and don’t need to test it each time.

As it says on http://php.net/manual/en/function.assert.php

Assertions should not be used for normal runtime operations like input parameter checks. As a rule of thumb your code should always be able to work correctly if assertion checking is not activated.

Unfortunately it is not written like that. Instead the two generators are put into place using setters as can be seen from the unit test which goes through the same prep process the dependency injector goes through. As to why they chose getters/setters over construct arguments, I simply don’t know. You are right, that would make the assert irrelevant since the error condition would be caught at object creation by the type hint. But I can’t pursue that approach - it would constitute an API change at a time when the API has been frozen unless the change is mandatory by outside forces (changes are being made for PHP 7 compat).

Yeah I can see why you needed it I was just hoping for a real world example of where assertions solved a problem that couldn’t be solved another way.

Here’s a contrived example:

class School {

	public function getAverageTestScore() {
		$total = 0;
		foreach ($this->students as $student) {
			$score = $student->getTestScore();
			assert($score >= 0 && $score <= 100);
			$total += $score;
		}

		return $total / count($this->students);
	}
}

The student object, even with return type hinting could return an integer outside the valid range and the assertion will pick up on it. But as I said, it’d be nice to find some real world examples where assertions are beneficial.

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.