Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:90352
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain basereality.com from 209.85.160.179 cause and error)
MIME-Version: 1.0
In-Reply-To: <CADMz3wco4JNLHQV+NvnaA5npF=hTnzp0zdB3gK+oDbL6tTDuFA@mail.gmail.com>
References: <CADMz3wco4JNLHQV+NvnaA5npF=hTnzp0zdB3gK+oDbL6tTDuFA@mail.gmail.com>
Date: Fri, 8 Jan 2016 14:56:04 +0000
Message-ID: <CA+kxMuR4kcDYMAiYjFPp0+anFRgYHiFbxmZcG2-x0yAGttiP-w@mail.gmail.com>
To: Dustin Wheeler <mdwheele@ncsu.edu>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC] Class Friendship
From: danack@basereality.com (Dan Ackroyd)

On 7 January 2016 at 20:25, Dustin Wheeler <mdwheele@ncsu.edu> wrote:
> Hello everyone,
>
> I have drafted an RFC that describes a proposal to add this feature to
> the language.
>
> https://wiki.php.net/rfc/friend-classes


Hi Dustin,

I try to avoid sounding too negative when people come up with ideas
that I might not agree with. However....I feel obliged to give honest
feedback earlier rather than later; I can't see any circumstances
under which I'd vote for this RFC.

In my dim and distant youth I was C++ programmer.....and friends
relationship between classes are not something I look back on fondly.
In fact, they were a source of much sorrow.

Although they solve a particular problem, they do so in a way that has
too many downsides. From the examples:

class Fibonacci {
    friend FibonacciTest;
    ....
}

class Person {
    friend HumanResourceReport;
    ...
}

The classes that are exposing their protected properties have to know
about what classes are going to be using them. It means if I want to
add a new test similar to the FibonacciTest, I also have to edit the
Fibonacci class itself.....this is pretty terrible.

That does not scale well - either in just the amount of code typed, or
in how difficult it is to reason about the code. What ends up
happening is that if a property needs to be exposed to friends once,
it ends up being exposed to lots of friends.

From the other example:

class HumanResourceReport
{
    public function getFullName() {
        // HumanResourceReport would not have access to protected
        // members of Person if not explicitly listed as a friend.
        return $this->person->firstName . ' ' . $this->person->lastName;
    }
}

Again, this has a massive downside. The 'domain logic' of how to
construct a full name from a first name and last name belongs in the
person class. By allowing the individual elements to be accessed, the
code is violating the 'separation of concerns'. When middle names are
added to the Person object, the HumanResourcesReport will continue to
generate full names as it has always done....until someone notices a
files a bug report.

I agree that we shouldn't put RFCs in a head-to-head fight. But to me
the package level protection of properties/methods sounds like a much
bette potential solution to the problem that currently visibility is a
binary option. But even if that idea wasn't floating around, I would
not like to see friendship based visibility in PHP, as the solution it
provides is worse than the problem it fixes, while at the same time
adding more complexity to the engine. Apologies again for being so
negative.

cheers
Dan

p.s. For the FibonacciTest example, if it was required to write a test
like that, below is how I would do it in PHP currently. It achieves
the aim of testing the class, without having to make the Fibonacci be
aware of the classes that are going to be testing it, or having to add
any new features to PHP.


class Fibonacci
{
    protected $previous;
    protected $current;

    public function __construct()
    {
        $this->previous = 0;
        $this->current = 0;
    }

    public function next()
    {
        $current = $this->current;
        $next = $this->previous + $this->current;

        if ($next == 0) {
            $next = 1;
        }

        $this->previous = $this->current;
        $this->current = $next;

        return $current;
    }
}

class IntrospectableFibonacci extends Fibonacci
{
    function getPrevious()
    {
        return $this->previous;
    }

    function getCurrent()
    {
        return $this->current;
    }
}


class FibonacciTest extends PHPUnit_Framework_TestCase
{
    public function testAssignmentAlgoForStateIsCorrect()
    {
        $fibo = new IntrospectableFibonacci();

        $this->assertEquals(0, $fibo->getPrevious());
        $this->assertEquals(0, $fibo->getCurrent());

        $n0 = $fibo->next();

        $this->assertEquals(0, $n0);
        $this->assertEquals(0, $fibo->getPrevious());
        $this->assertEquals(1, $fibo->getCurrent());
    }
}