Skip to content

Conversation

narfbg
Copy link
Contributor

@narfbg narfbg commented Mar 7, 2017

I often find myself writing checks similar to this one:

is_string($var) || (is_object($var) && method_exists($var, '__toString'))

So naturally I wanted a simpler way to get the same result and here it is. I'll try to reply in advance to what I think would be the common argument against this:

  • Scalar type hints did help a lot to eliminate the need for such checks, but don't get the same effect with strict_types=1.
  • This isn't limited to parameter validation.

I can imagine it not being a very valuable addition, but I also see no negative impact, so ... :)

When set to true, will allow objects implementing __toString() to pass is_string() validation
@Fleshgrinder
Copy link
Contributor

Fleshgrinder commented Mar 7, 2017

Thanks for your effort and initiative. 👍

However, I strongly believe that this is the wrong approach. Adding a flag to a function directly results in the fact that the function violates the single responsibility principle. What we actually need to make this work is a stringable pseudo-type like the iterable type that was introduced in PHP 7.1. This stringable pseudo-type is the union of the scalar primitive string and any class that implements the __toString method.

This has the advantage that we are actually able to use it together with strict_types, plus we have separate dedicated functions like is_stringable that adhere to the single responsibility principle. I actually wanted to create an RFC for that along with an implementation since iterable was accepted, but did not find the time yet.

Closing note: these pseudo-types are necessary in PHP because it has no coherent type system, and there is nothing we can do about this in short term. Hence, adding such pseudo-types is the only short term solution that we actually have.

PS: We could join forces on stringable if you want?

PPS: You can shorten your code to the following:

if (is_string($var) || method_exists($var, '__toString')) { // ...

@narfbg
Copy link
Contributor Author

narfbg commented Mar 8, 2017

Thanks for the great feedback.

I wouldn't mind joining forces on stringable in general, but I just mailed internals to see how everybody feels about it first (didn't have time to do that yesterday and you were quick to reply).

@Fleshgrinder
Copy link
Contributor

We will continue our discussion there. 😼

@nikic nikic added the RFC label Mar 9, 2017
@nikic
Copy link
Member

nikic commented Jun 2, 2017

Internals discussion for reference: http://markmail.org/message/uiuyhkhp7m7cxfke

If I remember correctly the reception to this change on internals was rather negative. Do you still plan to pursue this, or can this be closed?

@narfbg
Copy link
Contributor Author

narfbg commented Jun 5, 2017

I'd still like to see it happen, but apparently everybody else hates the idea ... don't mind if you close it.

@blar
Copy link
Contributor

blar commented Jul 4, 2017

I don't like a second argument in is_string but bool is_stringable($value) may be a solution.

@narfbg
Copy link
Contributor Author

narfbg commented Jul 5, 2017

@blar It's not.

This is all traceable in the internals discussion linked by Nikita, but here's the gist:

is_stringable() means "can it be cast to string" - every scalar can be cast to string.
What I want is "does it already have an explicit string representation", which includes only string type and the __toString() magic method.

Closing since nobody agrees with me on this.

@narfbg narfbg closed this Jul 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants