Namespaces internal refactoring

Also, there's a difference IMO between classes and class' properties.
Class details may be - indeed, need to be - hidden because this is what
the class is, one of it's reasons to exist - a tool to reduce complexity
by hiding irrelevant information and exposing only relevant information
as class' public API. You hide things not because you're afraid evil
users would abuse them - but as a service to the user to not overload
their mental models with irrelevant details and allow to deal with the
class by only considering the important things.

If it were only about suppressing unimportant details, documentation
would suffice, e.g. /** @private */ or the leading underscore
convention. I am, however, quite happy to have language support to
catch inadvertent accesses.

--
Christoph M. Becker

Hi Stas,

I'll comment your PS, since I'm the author of the PR.

On Sun, Aug 14, 2016 at 6:11 PM, Stanislav Malyshev smalyshev@gmail.com
wrote:

Hi!

• PHP 7 has private classes through anonymous/inner classes.

It's not exactly the same, and I suspect the same is true for Ruby. It's
true that anonymous classes can not be instantiated by other code. But
that is not what we were discussing here. This particular effect is
somewhat similar, the purpose of the feature is different.

Of course, some languages have them. Others don't. The point is saying
"it's not OO unless it implements $favorite_feature_of_mine" is not very
meaningful, at least at this point.

abstract class Data {
protected $id;
protected $name;
protected function __construct() {}
protected function __clone() {}
}

final class Entity extends Data {
public function getId() {
return $this->id;
}
public function getName() {
return $this->name;
}
}

final class Builder extends Data {
private $entity;
public function build() {
return clone $this->entity;
}
public function setId($id) {
$this->entity->id = $id;
}
public function setName($name) {
$this->entity->name = $name;
}
}

?>

I'm afraid I don't understand this code. Why both entity and builder are
Data? It looks like combining factory object with actual data object.
This is why __construct/__clone exists - so the object could take care
of the details and the factory didn't have to know about it in details.

I presume you want to say that Data object should be private class. I
think - at least from my partial understanding of the code - that it
shouldn't exist at all, especially given you don't want to expose it to
the user.

Another example that is not solvable with friend classes would be the
strategy pattern of hidden strategies as we find it for example in
Symfony's process component.

https://github.com/symfony/process/tree/master/Pipes

The pipes are purely internal to work around differences between PHP on
Unix and Windows. They are not meant for consumption through users, they

Sure, they aren't useful for most, but I see no problem in somebody
using them if necessary. Say, you may want to use Unix pipe on Windows
because you are in unix emulation environment, or you have a weird OS
that is not Unix and now Windows and want to reuse parts of these
classes to deal with it. There could be many cases we can't foresee when
we design it for our ideal use case.

At trivago we have hundreds of applications, packages, libraries and
developers who work in a very fast moving environment. Reading
documentation is not something most people spend time with.

I find so many things not right with this statement I don't know where
to start... So I just leave it with a note that having no time to do
things right usually means doing things wrong many times, and then
being forced to spend time to do things right because the whole jenga
tower crashes on your head. That is not a criticism of you or your
employer, but rather the result of experience learned the hard way. You
can get away with it from time to time, true, but it's just luck.

It's also easy to work around property and method access modifiers.
However, that is no argument to remove them now.

Not very easy. The only way to work around private is to use heavy
machinery like reflection. This is why, btw, private should be used very
carefully - when if you're sure nobody will need or is advised to
access that thing directly. Overuse of private, especially for methods,
leads to classes that are not extendable without horrendous amounts of
copypaste.

Also, there's a difference IMO between classes and class' properties.
Class details may be - indeed, need to be - hidden because this is what
the class is, one of it's reasons to exist - a tool to reduce complexity
by hiding irrelevant information and exposing only relevant information
as class' public API. You hide things not because you're afraid evil
users would abuse them - but as a service to the user to not overload
their mental models with irrelevant details and allow to deal with the
class by only considering the important things.

However, if we're talking about collection of classes sharing a
namespace, there's no such thing as "namespace's API", at least not in
common use. There's a concept of library's API, but it's not derived
from classes alone - it's usually what documentation does, while class
API can be yet figured from it's methods, library API is usually
impossible to figure out just looking at the classes - you need the next
level, documentation.
So in my opinion, if you hide some classes from instantiation from
outside namespace (which by itself carries a lot of assumptions, such as
every class that will ever need it will share the same namespace),
reduction in complexity is almost none, since nobody uses "list of
available classes in package/namespace" as a way to work with the
package anyway.

P.S. BTW I still don't know why you need to refactor namespaces to have
namespace-private classes anyway. Isn't it just a matter of the match
between current scope and target class name?

I wanted to come up with a solution before writing an RFC.
The problem that is not yet solved in that PR is the check of instantiation
without being scoped. Example:

<?php

namespace Foo;

$bar = new ThisIsAPrivateClass();

This should work... but this code:

<?php

$bar = new \Foo\ThisIsAPrivateClass();

Should break.
The problem is that opline have no scope context, so at both of them the
scope = NULL.

We could easily add a scope every time a namespace is declared and remove
when ended, but as Nikita highlighted (here:
https://github.com/php/php-src/pull/947#issuecomment-224934615 ), this is
not the correct solution.

I hit a dead end on that PR. Changing opline (or oparray) to support a new
struct _zend_namespace is over my internals knowledge. I'd love if someone
could take over and complete it. I can write the RFC while someone else
finalize the code.

--

Stas Malyshev
smalyshev@gmail.com

--

--
Guilherme Blanco
Lead Architect at E-Block

Hi!

• PHP 7 has private classes through anonymous/inner classes.

It's not exactly the same, and I suspect the same is true for Ruby. It's
true that anonymous classes can not be instantiated by other code. But
that is not what we were discussing here. This particular effect is
somewhat similar, the purpose of the feature is different.

It is part of what we are discussing.

Of course, some languages have them. Others don't. The point is saying
"it's not OO unless it implements $favorite_feature_of_mine" is not very
meaningful, at least at this point.

I never said that I agree with that particular comment since you are
completely right that this is not true. Visibility modifiers do not
require OO and OO does not require visibility modifiers.

abstract class Data {
protected $id;
protected $name;
protected function __construct() {}
protected function __clone() {}
}

final class Entity extends Data {
public function getId() {
return $this->id;
}
public function getName() {
return $this->name;
}
}

final class Builder extends Data {
private $entity;
public function build() {
return clone $this->entity;
}
public function setId($id) {
$this->entity->id = $id;
}
public function setName($name) {
$this->entity->name = $name;
}
}

?>

I'm afraid I don't understand this code. Why both entity and builder are
Data? It looks like combining factory object with actual data object.
This is why __construct/__clone exists - so the object could take care
of the details and the factory didn't have to know about it in details.

I presume you want to say that Data object should be private class. I
think - at least from my partial understanding of the code - that it
shouldn't exist at all, especially given you don't want to expose it to
the user.

You need to imagine that the entity has more properties of course and
no, the data class should not exist at all since it is only used to
emulate something that is currently simply impossible: allow some class
access to parts of another class while not allowing anyone else access
to it.

Again our goal:

Create a builder that builds an immutable entity.

A possible solution as I could imagine it:

<?php

// This is the magic that enabled backwards compatibility and changes
// semantics of leaving the visibility modifier out.
(assembly|group|module|package|...) Fleshgrinder\Internals;

namespace Examples\Builder;

public final class Entity {

//const class = 'Fleshgrinder\Internals\Examples\Builder\Entity';

number $id;
string $name;
DateTimeImmutable $created;
DateTimeImmutable $changed;
SomeValueObject1 $some_property_1;
SomeValueObject2 $some_property_2;
// ...
SomeValueObjectn $some_property_n;

function __construct() {}
function __clone() {}

// Getters for everything ...

}

public final class EntityBuilder {

private Entity $entity;

public function __construct() {
    $this->entity = new Entity;
}

public function build() {
    return clone $this->entity;
}

// Setters for everything ...

// Other useful stuff while building ...

}

?>

Currently the only way to achieve this is through an interface for the
entity and hoping that everyone uses that interface and never finds out
that the class actually offers much more functionality. Or you add a
shit load of arguments to the constructors. Or, my preferred approach,
via the magic __set_state method that is not auto-completed via the IDE.
That approach is still not ideal but one of the best. The problem is
users who find out that that __set_state method exists and they might
start using it. The last resort are hacks ... more about that later.

Another example that is not solvable with friend classes would be the
strategy pattern of hidden strategies as we find it for example in
Symfony's process component.

https://github.com/symfony/process/tree/master/Pipes

The pipes are purely internal to work around differences between PHP on
Unix and Windows. They are not meant for consumption through users, they

Sure, they aren't useful for most, but I see no problem in somebody
using them if necessary. Say, you may want to use Unix pipe on Windows
because you are in unix emulation environment, or you have a weird OS
that is not Unix and now Windows and want to reuse parts of these
classes to deal with it. There could be many cases we can't foresee when
we design it for our ideal use case.

The joke is, you cannot do that. Both the Process and the ProcessBuilder
do not allow you to change the Pipes that are in use. Unless you hack it.

https://github.com/symfony/process/blob/master/Process.php
https://github.com/symfony/process/blob/master/ProcessBuilder.php

These are purely internal classes that serve a single purpose.

At trivago we have hundreds of applications, packages, libraries and
developers who work in a very fast moving environment. Reading
documentation is not something most people spend time with.

I find so many things not right with this statement I don't know where
to start... So I just leave it with a note that having no time to do
things right usually means doing things wrong many times, and then
being forced to spend time to do things right because the whole jenga
tower crashes on your head. That is not a criticism of you or your
employer, but rather the result of experience learned the hard way. You
can get away with it from time to time, true, but it's just luck.

Thoughtful API design is bad? Pretty much any- and everything in PHP is
a breaking change because many things cannot be restricted. The addition
of visibility modifiers to constants in PHP 7.1 is a blessing because as
of now any change to a constant is directly a breaking change. This
sucks and only helps to increment the MAJOR part of our SemVer version
strings. Of course it also means that we need to touch a bunch of
packages because our composer is restricted to ^1.0 and now needs to be
^2.0.

It's also easy to work around property and method access modifiers.
However, that is no argument to remove them now.

Not very easy. The only way to work around private is to use heavy
machinery like reflection. This is why, btw, private should be used very
carefully - when if you're sure nobody will need or is advised to
access that thing directly. Overuse of private, especially for methods,
leads to classes that are not extendable without horrendous amounts of
copypaste.

This is not true, closures are a lightweight work around here.
Nonetheless, heavy usage of private for methods does not make much sense
in any context or programming language but it does for any other kind of
member (we are left with constants and properties).

Also, there's a difference IMO between classes and class' properties.
Class details may be - indeed, need to be - hidden because this is what
the class is, one of it's reasons to exist - a tool to reduce complexity
by hiding irrelevant information and exposing only relevant information
as class' public API. You hide things not because you're afraid evil
users would abuse them - but as a service to the user to not overload
their mental models with irrelevant details and allow to deal with the
class by only considering the important things.

Now exchange class with package in the above paragraph and all of this
is equally true.

However, if we're talking about collection of classes sharing a
namespace, there's no such thing as "namespace's API", at least not in
common use. There's a concept of library's API, but it's not derived
from classes alone - it's usually what documentation does, while class
API can be yet figured from it's methods, library API is usually
impossible to figure out just looking at the classes - you need the next
level, documentation.
So in my opinion, if you hide some classes from instantiation from
outside namespace (which by itself carries a lot of assumptions, such as
every class that will ever need it will share the same namespace),
reduction in complexity is almost none, since nobody uses "list of
available classes in package/namespace" as a way to work with the
package anyway.

A namespace might not have any relevance for you but it has to the PHP
community at large who are using PSR-0/4 and Composer. All of my
namespaces start with Fleshgrinder and all of trivago's namespaces start
with Trivago. The same is true for Symfony, Drupal, Laravel, Zend, ...
so yes, my classes share the same namespace and they will always share
the same namespace unless someone else starts using my namespace.

Also your last paragraph is completely untrue. Every time one interacts
with the IDE auto-completion a listing of all available declarations
that fit the context is given. Hence, if I type new followed by a
Ctrl + Space everything is being listed. The only thing I can do now is
adding a filter by starting to type a few characters.

Your approach of making everything public from the very beginning means
that you add everything to your public API. It also means that you need
to support everything in a backwards compatible manner. What does that
mean for us as library developers?

Traits are the worst since we cannot change anything.

Interfaces cannot be changed in any way other than moving members up
into another interface that we extend but the other interface is not
allowed to add any other members to our interface.

Classes cannot be changed in any way because someone might extend them.
We cannot introduce new members since extending classes might already
have them. We can however introduce new private things and fiddle around
with magic methods to some extend.

Final classes give one more freedom but are a pain for people who like
to rely on mocking and learned what interface explosion means.

It is about API design and not only about class API design. It is a
combination of documentation and modifiers.

P.S. BTW I still don't know why you need to refactor namespaces to have
namespace-private classes anyway. Isn't it just a matter of the match
between current scope and target class name?

This is better explained by nikic and others.

--
Richard "Fleshgrinder" Fussenegger

Hey Stas

Hi!

A realization that needs to be made is that beginners would be using
libraries that requires to make valid restrictions, preventing those
beginners to mess up with code they shouldn't. So even if the use case
is only valid for 0.01% of code producers, it might be valid for 20%+ of
consumers.

I don't think PHP needs more bondage-and-discipline. If beginners "mess
up with code they shouldn't" that is usually, in my experience, a sign
of library authors not serving their use case well, and not implementing
functionality that they need. Either because they didn't know, or
because their opinion about user's needs differs from user's opinion
about their needs. In this case, I'm on the user's side. Hacker's ethos
is doing things other people think can't be done.

You don't think so, but this happened many times also with non-beginners
(what Guilherme was talking about).
Conventions are not sufficient to keep a codebase consistent and safe:
explicit rules do that.

Hi Stas,

On Sun, Aug 14, 2016 at 6:35 PM, Stanislav Malyshev smalyshev@gmail.com
wrote:

Hi!

A realization that needs to be made is that beginners would be using
libraries that requires to make valid restrictions, preventing those
beginners to mess up with code they shouldn't. So even if the use case
is only valid for 0.01% of code producers, it might be valid for 20%+ of
consumers.

I don't think PHP needs more bondage-and-discipline. If beginners "mess
up with code they shouldn't" that is usually, in my experience, a sign
of library authors not serving their use case well, and not implementing
functionality that they need. Either because they didn't know, or
because their opinion about user's needs differs from user's opinion
about their needs. In this case, I'm on the user's side. Hacker's ethos
is doing things other people think can't be done.

It's a valid point of view, indeed.
But thinking from a different perspective, if you are using a tool that
does X, users shouldn't rely on Y to complete their job. Specially if Y is
already on a namespace marked as "Internal" with @private annotation and a
huge capital disclaimer that it was a private class.

It doesn't matter how much documentation you add, without language tooling
that prevents to use X as Y, all you can expect are weird use cases and
breakages, that re either not intended to work or are just not supposed to
be done at all. Like I always say "never underestimate someone's ignorance".

It's about defensive programming.
I never knew TopologicalSorter was used by several users until I
actually broke its API. Imagine the frustration of all those developers
with tight deadlines having to deal with a sudden breakage, because the
language it too open to notify them about something shouldn't be used.

I'd rather say "imagine the frustration of all those developers that
rely on the library authors who are oblivious to their true needs and
still the developers at the mercy of them every day". I agree that can
be frustrating. The solution is not more detachment from the needs of
the users and more fencing out so these pesky users would have
absolutely no way to use my code in ways that I didn't foresee, because
I'm capable of foreseeing 100% of user's needs and if I'm not, it's
their fault. At least, I don't think it is.

Taking from this perspective (and also inline with your initial comment),
what you are suggesting is basically break down common, reusable piece of
functionality on its own package. Yes, that is valid, and I have nothing
against it.
The lack of breaking X into smaller packages (such as decoupling Y) doesn't
mean users should use Y at will and suddenly expect/blame about API breaks.

Every time someone questions me about the validity of private classes, I
like to exercise replacing the term "private" with "final". Final is
part of PHP language and it also there's non application that is
prevented without it.

I think most of the usages of final are wrong too, and for the same
reasons. It's a claim you have foreseen all user's needs and they'd
never need anything else. It's usually wrong.

modifier, https://mwop.net/blog/2012-06-28-oop-visibility.html history
would be completely different.

There Matthew says pretty much what I say, so I agree:

I feel that frameworks and libraries should use private and final
sparingly.

Here you basically agreed with everything I said. I'm not saying every
single class should be private (or final), but you just agreed with Matthew
and I that there are valid use cases for each one.

That's not about preventing for no reason and it should be bypassed.
It's about somehow telling users that something shouldn't be used at all.
The amount of people that takes the wrong approach is insane, and any
kind of prevention to do it is beneficial.

If the amount of people using your library in a wrong way is insane,
maybe something needs to be changed in the library? ;) If you build a
nice slick walkway and people keep walking over the grass 100 meters
aside, it may just be you've built the walkway in a wrong place.

It's always a balance between what you can do in such a small spare time
and benefits you get out of these contributions that you could use to get
your job done.
I could perfectly decouple 10 sub-packages out of ORM, but that only brings
more maintenance burden that will consume more time that I already don't
have.

I managed big teams of development where I work, and controlling code
produced by 40+ developers is not easy.
Countless classes are marked as final and I bet hundreds would be final
if I could.
Handling a company source code is easy through code reviews, and also
walk a few steps to ask why a certain class is final or (hypothetically,
private).

Maybe it makes sense to a company which works on its own product, never
to be used by others, and has every use of the code tightly controlled,
foreseen and signed in triplicate with TPS reports on top. My opinion
though it is more of a burden than a benefit to an open-source
environment built on extending and modifying existing code.

Not really. I worked as Chief Architect and controlling integration
conflicts consumed most of my time while designing and planning the
platform.
Every feature had critical technical architecture points highlighted and
another series of decisions to be made by developers. When handling large
volumes of traffic, some technology decisions are required in order to keep
business online, and some critical pieces of code were heavily controlled
by key developers.

I've walked through several scenarios of peers coming to my desk asking
to remove final because reasons and going out with a much simpler
solution without the need to remove it.

That implies you are always smarter than any user of your classes and
have foreseen their need in advance before they even were aware of them.
It is nice to be a genius, unfortunately, there's only one of you, and
maybe another dozen of similar geniuses in existence (I'm certainly not
one of them, not even close). Others need to deal with situations where
their code is used in situations where they did not foresee 100% of
users' needs and some modification/extension should be made. Betting on
being a prophetical genius is usually a losing bet. Unless, of course,
you are. But I would rather prefer the flexibility of being able to
correct my mistakes and shortsightedness than betting on that. Which
means flexibility beats bondage-and-discipline.

By far I'm not the most genius person I've worked with. I tend to relate
this to scope knowledge and the amount of time invested during strategy,
architecting and planning a solution. If I spend 100h exercising edge cases
out of a solution, I may have covered a substantial amount of potential
problems that may come up by developers who are purely focused on
delivering a small feature.
That doesn't mean I have covered 100% of all edge cases, but surely I've
exercised things out of a solution more than others. If I haven't, I
wouldn't be doing my architect's job.

Anyway, this is already derailing into a completely separate topic. Let's
stick to the main subject, please.

Problem of lack of Annotations support... I've tried several times, and
made sure I was accessible enough to answer every situation or scenario
Doctrine faced and reasons for every decision taken on the library.
Still, people seem to ignore expertise and decide to take their own
methods which only solves (I'll use your numbers now) 0.01% of use cases.

Unfortunately I am of opposite opinion - that the Doctrine needs -
important as they are - are 0.01% of all PHP user's needs. And the fact
that we don't have solution we could build on (again, you see the
leitmotif I'm harping on - being flexible instead of trying to be ideal
at the start and freezing there) because that solution won't serve all
the needs appears very unfortunate to me.

Doctrine needs seems like a 0.01% of use cases, but they are also shared by
Symfony, Zend Framework and many other projects too.
We even downscaled a lot the suggested solution too, but it was ignored
somehow. For example, typed properties would reduce significantly our
implemented solution, because we'd rely on language support to the work for
us.

The only problem I find is that PHP is full of half-baked solutions. People
focus too much on a specific feature and do not consider a holistic
approach. Small example: people found it seriously cool to adopt ?Foo as a
nullable type. However, it complicated a lot the implementation of typed
properties (Joe had to work on a triple variable state to make it work).
Now implementation of typed properties was to blame, but I seriously think
it was the introduction of nullable types the culript.

When I mentioned allowing to assess imported classes, I mean this:

namespace Foo;

use Bar\Woo;

/**

• @Woo\Fuzz()
  */
  class Lala {}

Annotation parser needs to understand what was "use"d, so it can
properly refer to FQCN. That way, we need to somehow discover something

I see what you mean. This seems to be a problem because you are trying
to do compiler things without the support of the compiler. I'd say the
right solution is to let the compiler do these things...

Yes, yes and yes. When can we get that?
I'll again reinforce that library developers have to come up with creative
ways to handle lack of support in the language. That was just one example.
Instead, the language should offer proper support and let library
developers to come up with creative tools (and now creative ways) to
improve and ease end user's development.

--
Stas Malyshev
smalyshev@gmail.com

--
Guilherme Blanco
Lead Architect at E-Block