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('<?php echo "foo";'));
This usecase might not be as common as the one where we want to tokenize
a file's content, thus, we do not add a short-hand for it. However, in
case it would be so common that it makes sense to add, we should add it.
final class Lexer {
// Same as before...
public static function fromString(string $source): self {
new static(new Cursor($source));
}
}
Once again, no logic, no branching, no hardcoded strategies. It does one
thing and one thing only: sparing us from re-typing the same construct
over and over again. DRY is one of the most important principles of them
all after all.
Another question to ask before adding such methods lightly is whether we
can justify the additional dependency. Depending on `File` and `Path` is
most certainly never a problem, after all, we need the file system at
all times. The `Cursor` on the other hand might be debatable, as it
might reside in a special component that is not necessarily available
just because the `Lexer` is.
I could go on and on and on now but I'm getting kind of off topic now. 😉
--
Richard "Fleshgrinder" Fussenegger
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
