Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:98627 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 27966 invoked from network); 24 Mar 2017 20:34:19 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 24 Mar 2017 20:34:19 -0000 Authentication-Results: pb1.pair.com smtp.mail=php@fleshgrinder.com; spf=permerror; sender-id=unknown Authentication-Results: pb1.pair.com header.from=php@fleshgrinder.com; sender-id=unknown Received-SPF: error (pb1.pair.com: domain fleshgrinder.com from 212.232.28.122 cause and error) X-PHP-List-Original-Sender: php@fleshgrinder.com X-Host-Fingerprint: 212.232.28.122 mx201.easyname.com Received: from [212.232.28.122] ([212.232.28.122:58266] helo=mx201.easyname.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id 13/5A-40046-8C285D85 for ; Fri, 24 Mar 2017 15:34:18 -0500 Received: from cable-81-173-135-7.netcologne.de ([81.173.135.7] helo=[192.168.178.20]) by mx.easyname.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1crVuX-0008BE-FA; Fri, 24 Mar 2017 20:34:13 +0000 Reply-To: internals@lists.php.net References: <16.06.40046.20A35D85@pb1.pair.com> <153a6a0f-1c22-6560-8bca-584e92915840@fleshgrinder.com> To: Niklas Keller , PHP Internals Message-ID: <43536364-e490-83ec-b230-faadf5c706fb@fleshgrinder.com> Date: Fri, 24 Mar 2017 21:34:06 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-DNSBL-PBLSPAMHAUS: YES X-DNSBL-SURBL: YES Subject: Re: [PHP-DEV] Re: TOKEN_AS_OBJECT for token_get_all() From: php@fleshgrinder.com (Fleshgrinder) On 3/24/2017 8:15 PM, Niklas Keller wrote: > If PhpToken::getAll() violates the SRP, then Lexer::fromFile() is just the > same sort of violation. > > Regards, Niklas > This is kind of true in PHP, because we are lacking proper abstractions for path and file handling. It is most certainly not true in many other languages. Usually such named constructors or factory methods are just short-hands to trigger a series of other behavior that abides the SRP. throws InvalidPathException throws IOException public static function fromFile(string $path): self { return new static(new File(new Path($path))->getContents()); } There is not logic at all inside this method, it is only there for convenience to spare you from writing this over and over and over and over ... again. In the PHP case the whole thing directly explodes in complexity: public static function fromFile(string $path): self { // Already questionable if we can actually pass that string // to file_get_contents since it might be a remote file, or // complete garbage that does not even remotely resemble a // path ... who knows ... $contents = file_get_contents($path); if ($contents === false) { // Let's just hope there is no other error in here from // a previous error ... $error = error_get_last(); // ... } return new static($contents); } I shortened the whole thing because it is to cumbersome to write all of this without an IDE. So yes it violates the SRP in classic PHP, but no it does not necessarily violate SRP per se. Also note that adding `Token::getAll` is more than weird from a logical point of view too. Since `Token` represents a single thing, whereas `getAll` by logic belongs to some kind of collection of things. Note that if we go further with the whole thing that we wouldn't even want to deal with a string in the constructor, but rather a `Reader`, `BufferedReader`, or `SeekableReader`. A `File` instance most certainly would comply with that: final class Lexer { public function __construct(SeekableReader $source); public static function fromFile(string $path): self { return new static(new File(new Path($path))); } } Now in case we want to work with a string directly, e.g. unit tests, we would need a seekable reader that encapsulates that string, e.g. a `Cursor`. new Lexer(new Cursor('