Everything

php

optionally return class name from gettype

php.internals

Timothy Younger

10 years ago

hi Internals Team,

i find myself writing this line frequently in projects:

is_object($value) ? get_class($value) : gettype($value);

it seems i'm not the only one:

https://github.com/search?l=php&q=get_class+gettype&ref=searchresults&type=Code&utf8=%E2%9C%93

although it is small, it amounts to a lot of duplicated logic in need of
testing. i'm looking to write that line, and its tests, one last time.

i think it's a bit trivial to warrant a standalone lib, but might already
exist or fit in some helper package.

instead, what do you think about an optional param for the gettype method
to provide this functionality? here's the new (hopefully
backwards-compatible) behavior:

var_dump(gettype(new \stdClass(), true));
string(8) "stdClass"

var_dump(gettype(new \stdClass()));
string(6) "object"

here's my commit, proposed for php/php-src:

https://github.com/abacaphiliac/php-src/commit/f4e2fae9145cec5af94f197acf71dae02cd61b08

and my commit's build on travis:

https://travis-ci.org/abacaphiliac/php-src/builds/148218512

any interest? if gettype isn't the right home for this logic, then perhaps
someplace else?

thank you.
Tim Younger

Ryan Pallas

10 years ago

I love this idea (as I've written much the same code before), I do think it
will require a full RFC though.

On Fri, Jul 29, 2016 at 7:53 AM, Timothy Younger <abacaphiliac@gmail.com>
wrote:

Niklas Keller

10 years ago

2016-07-29 16:03 GMT+02:00 Ryan Pallas <derokorian@gmail.com>:

> I love this idea (as I've written much the same code before), I do think it
> will require a full RFC though.
>

I don't think it does require a RFC, it's not a language change. Let's see
if anyone opposes first.

I'm not sure on the boolean through, I think a new function might be better.

It's something that can be implemented in userland, but it's also something
that's really often needed.

Regards, Niklas

David Rodrigues

10 years ago

Niklas Keller wrote:

> I don't think it does require a RFC, it's not a language change. Let's see
> if anyone opposes first.

I agree to, if the internals agree, I guess that RFC is needless.

Niklas Keller wrote:

> I'm not sure on the boolean through, I think a new function might be better.

In this point I desagree.
I think that boolean is the best way, mainly to avoid a new function
on userland.
It's like an "extension" for gettype(), and make senses just extend it.


2016-07-29 11:21 GMT-03:00 Niklas Keller <me@kelunik.com>:

> 2016-07-29 16:03 GMT+02:00 Ryan Pallas <derokorian@gmail.com>:
>
>> I love this idea (as I've written much the same code before), I do think it
>> will require a full RFC though.
>>
>
> I don't think it does require a RFC, it's not a language change. Let's see
> if anyone opposes first.
>
> I'm not sure on the boolean through, I think a new function might be better.
>
> It's something that can be implemented in userland, but it's also something
> that's really often needed.
>
> Regards, Niklas

--
David Rodrigues

Fleshgrinder

10 years ago

You can copy together the code for this from here including test cases:

https://github.com/php/php-src/pull/1932
https://github.com/php/php-src/pull/1935
https://github.com/php/php-src/pull/1957

--
Richard "Fleshgrinder" Fussenegger

Niklas Keller

10 years ago

>
> Niklas Keller wrote:
> > I'm not sure on the boolean through, I think a new function might be
> better.
>
> In this point I desagree.
> I think that boolean is the best way, mainly to avoid a new function
> on userland.
> It's like an "extension" for gettype(), and make senses just extend it.

The issue is that it's not clear what this boolean means just from reading
the code.
If you follow Clean Code, you shouldn't have something like that, only in
very, very rare cases.

Rasmus Schultz

10 years ago

I agree, an argument that essentially turns it into a different function is
not a good practice.

Suggestions for a function-name?

typeof() or vartype() maybe?

On Fri, Jul 29, 2016 at 8:17 PM, Niklas Keller <me@kelunik.com> wrote:

Timothy Younger

10 years ago

thank you all for your feedback. it seems this thread is split between an
optional param and a new function, so i created a new function called
var_type for comparison.

commit:
https://github.com/abacaphiliac/php-src/commit/eca6f77bf2744c79671d1dfbb641b753503d4a1a

build:
https://travis-ci.org/abacaphiliac/php-src/builds/157555638

it mirrors gettype except that it returns a classname instead of the string
"object". i'm not a fan of the code duplication, so i'm looking into a
couple of ways to address that. the result will likely couple var_type to
gettype, but that is no worse than my optional param solution. is decoupled
code better than code duplication in this scenario?

does comparing my commits help? how can i proceed? should i open merge
requests that reference each other to see if either one is acceptable, or
if they need work? should i submit an RFC that includes both commits so
that there can be a vote?

thanks for your time and honest feedback.

On Sat, Jul 30, 2016 at 5:46 AM, Rasmus Schultz <rasmus@mindplay.dk> wrote:

Timothy Younger

10 years ago

i just thought of another option. what if gettype returned the classname as
a part of the "object" string, e.g. "object (point)", "object (stdClass)",
"object (Exception)", etc.

this would be similar to gettype's return on a closed resource: "resource
(closed)":
https://github.com/php/php-src/blob/master/ext/standard/type.c#L71

although, this could break consumer code. e.g. gettype($obj) === "object".
maybe this option is not so good.

On Mon, Sep 5, 2016 at 12:02 AM, Timothy Younger <abacaphiliac@gmail.com>
wrote:

Fleshgrinder

10 years ago

On 9/5/2016 9:02 AM, Timothy Younger wrote:

> thank you all for your feedback. it seems this thread is split between an
> optional param and a new function, so i created a new function called
> var_type for comparison.
>
> commit:
> https://github.com/abacaphiliac/php-src/commit/eca6f77bf2744c79671d1dfbb641b753503d4a1a
>
> build:
> https://travis-ci.org/abacaphiliac/php-src/builds/157555638
>
> it mirrors gettype except that it returns a classname instead of the string
> "object". i'm not a fan of the code duplication, so i'm looking into a
> couple of ways to address that. the result will likely couple var_type to
> gettype, but that is no worse than my optional param solution. is decoupled
> code better than code duplication in this scenario?
>
> does comparing my commits help? how can i proceed? should i open merge
> requests that reference each other to see if either one is acceptable, or
> if they need work? should i submit an RFC that includes both commits so
> that there can be a vote?
>
> thanks for your time and honest feedback.

You need to create a PR and an RFC. I include some links for you from my
original attempt to create such functionality:

- https://github.com/php/php-src/pull/1932
- https://github.com/php/php-src/pull/1935
- https://github.com/php/php-src/pull/1957

You see that I always create tasks for myself in the PR messages of what
needs to be done. You will also see that the RFCs were all negative,
however, you can count on my support for a second attempt of creating
var_type if you really want to go forward with this.

My personal opinion is of course to fix the issues that gettype
currently has altogether with the new var_type function, as explained in
detail in my original RFC.

Have a look at my implementation of var_type which has smaller tests
that are more useful and how it uses interned strings for better
performance. (The class name functionality is found in the var_info PR.)

On 9/5/2016 9:35 AM, Timothy Younger wrote:

> i just thought of another option. what if gettype returned the classname as
> a part of the "object" string, e.g. "object (point)", "object (stdClass)",
> "object (Exception)", etc.
>
> this would be similar to gettype's return on a closed resource: "resource
> (closed)":
> https://github.com/php/php-src/blob/master/ext/standard/type.c#L71
>
> although, this could break consumer code. e.g. gettype($obj) === "object".
> maybe this option is not so good.
>

Touching the output of gettype() is a bad idea. Although it will
actually be broken in 7.2 the decision to do so was not voted on, it was
simply done. :(

https://github.com/php/php-src/pull/2022

--
Richard "Fleshgrinder" Fussenegger