On Mon, 18 Nov 2019 at 00:18, Mike Schinkel <m...@newclarity.net> wrote:
> I now realize that my commenting on my experience in reviewing legacy code
> — where long names are frequently used, regardless of the fact they are not
> required — caused you to focus on the long naming comment aside and not on
> the primary ask for consistency.
>
> Even if short, a code base littered with method names like toV5Json() or
> getV5Array() or formatForJsonV5() is still inconsistent.
>
>
Inconsistent with what? Unless you suggest we introduce enough magic
methods that every method name in your application begins with two
underscores, you are always going to have to name things.
If there is a common requirement for objects to be converted to a
particular type of array, then you should pick a standard name for that and
enforce it via code review; or better, make all those objects implement an
interface, and the compiler will enforce it for you.
That seems no harder to me than enforcing a convention that __toArray
should always be included, and have a particular meaning.
> What I was suggesting instead, which obviously was not clear, is that you
> communicate the *purpose* with the namespace and by doing so allows for the
> capability of casting to array be added. Otherwise the perfect is the
> enemy of the good.
>
>
I'm not asking for any kind of perfection, I'm just saying names should be
meaningful. Bear in mind that namespaces are really just part of the class
name, so all you're really saying is that you like long specific class
names and short generic method names.
> Similarly it is not clear to me was toV5Json() or getV5Array() or
> formatForJsonV5() mean.
>
>
I was imagining an API that had gone through different versions, so needed
methods to serialize objects into the JSON published in version 5 of a
specification.
how do I make use of these other classes? Would I have to call "(array)(new
> \Widgets\Mustache\Widget($myWidget))", as sugar for "(new
> \Widgets\Mustache\Widget($myWidget))->__toArray()"?
>
>
> Maybe we code differently, but I would never write a full namespace in
> code like that. I would have one of the following at the top of my PHP file:
>
>
Yes, that example was just me being lazy when typing the example, sorry. I
was trying to understand what the "MustacheWidget" class did, and it seems
I was correct in seeing it as an adapter.
> If so, I don't really see the benefit of the magic method over just
> standardising a method name, like "interface MustacheFormatter { public
> function getData(): array; }"
>
>
> Backward compatibility is the benefit. If we standardize one any name
> then we can break existing code.
>
>
If your code base is such a mess that you can't propose any method name
without it conflicting, and can't trace usages of any method that needs
renaming to not conflict, then yes, you get a one-off benefit from the fact
that __ is reserved. That seems a pretty poor justification for a language
feature.
Since you're suggesting separate classes for each transform, how many
public methods do they have? Would making everything in your *\Mustache\*
namespaces implement a MustacheFormatter interface actually be harder than
adding a __toArray method to them all?
One huge benefit I haven't mentioned yet is the ability to add parameters.
Imagine if your mustache formatters are used in both plain text and HTML
contexts, and that affects some of the data to return. If you have a
MustacheFormatter interface, you could alter it to require "getData(bool
$forHtml=false)", and only change the implementations where it mattered.
With __toArray(), you are limited to one parameter-less method per class,
so would have to create new copies of every single formatter, in a new
MustacheHtml namespace.
Thinking about it, if you're only going to allow one method per class, with
the class's namespaced name telling you what it does, you might as well use
__invoke():
use Foo\Mustache\Widget as MustacheWidget;
$tplWidget = new MustacheWidget($myWidget);
$output = $tplWidget(); // calls Foo\Mustache\Widget::__invoke()
> Because asArray() and somethingMoreSpecific() are neither a standard you
> don't get polymorphism with different named methods.
>
>
You can only rely on __toArray if you standardise on every object
implementing it; at which point, you can standardise on any name you like.
People are quite happily using polymorphism with named methods in literally
millions of OO code bases.
> And any new standard non-magic method we add has the potential to break
> BC. And is unlike anything else in PHP I can think of.
>
>
I'm not suggesting *PHP* standardises on some other name, just that *your
code base* standardises on common names for common tasks.
> OTOH, I have a hard time thinking of a scenario where __toArray() would
> actually cause a problem for someone like you who cannot think of a
> benefit. Can you present a use-case how it would cause you a tangible
> problem that *could not be resolved* with namespaces or by just creating
> your own somethingMoreSpecific() method and ignoring __toArray()?
>
>
Well, I started this sub-thread saying I was "sceptical" rather than
"strongly opposed". If the feature was added, I would simply ignore it, and
probably argue against its use in code review and style guides.
However, other people have pointed out that unlike (string)$object,
(array)$object does have a default behaviour, and adding an overload for it
has the potential to break code relying on that. So it's not an entirely
zero-cost feature.
Regards,
--
Rowan Tommins
[IMSoP]
