On 8/15/2016 12:11 AM, Stanislav Malyshev 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.
> 

It is part of what we are discussing.

On 8/15/2016 12:11 AM, Stanislav Malyshev wrote:
> 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.

On 8/15/2016 12:11 AM, Stanislav Malyshev wrote:
>> 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.

On 8/15/2016 12:11 AM, Stanislav Malyshev wrote:
>> 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.

On 8/15/2016 12:11 AM, Stanislav Malyshev wrote:
>> 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.

On 8/15/2016 12:11 AM, Stanislav Malyshev wrote:
>> 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).

On 8/15/2016 12:11 AM, Stanislav Malyshev wrote:
> 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.

On 8/15/2016 12:11 AM, Stanislav Malyshev wrote:
> 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.

On 8/15/2016 12:11 AM, Stanislav Malyshev wrote:
> 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

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to