Ticket Details
Type-hint arrays in constructors.
ENHANCEMENT Ticket (closed)
{{{
[15:58] scoates:
curious: why public function __construct($config = array()) { and not public function __construct(array $config = array()) { ?
[15:58] scoates:
you even cast it to an (array) a couple lines down just to be sure
[15:58] scoates:
(I suppose this isn't directly related to couch, but to Http, or another parent)
[15:59] gwoo:
i heard that kind of casting was slower ;)
[15:59] gwoo:
i would be up for changing it if you think it's worth it
[15:59] scoates:
hmm. I hadn't head that, but I'm not concerned so much about speed as that it would make more sense to typehint an array if it's always used as an array
[16:00] scoates:
well, I'm not even concerned; just seems a bit strange.
[16:00] gwoo:
yeah every constructor in lithium is an array
[16:00] scoates:
right
[16:01] _nate_:
scoates: Maybe I'm showing my ignorance here, but I didn't know you could type-hint not-objects.
[16:01] scoates:
only array
[16:01] scoates:
objects by class and array
[16:02] _nate_:
Well that's useful. That would actually allow us to get rid of quite a few typecasts.
[16:02] _nate_:
scoates: The problem, which you've probably noticed, is that we concatenate arrays with += a lot, and if you accidentally throw in a stray null somewhere, you get a fatal error.
[16:03] scoates:
that's a good point. let me check something.
[16:04] scoates:
php> function a(array $a=array()) {}
[16:04] scoates:
php> a(null)
[16:04] scoates:
Catchable fatal error: Argument 1 passed to a() must be an array, null given
[16:05] scoates:
_nate_: even if it didn't work, it's a code-time error, not an input-based error, so fatal is fine, IMO
[16:06] _nate_:
scoates: True, but occurrences and especially causes can be non-obvious. Which makes adding the type-hint so great, because you can see exactly what happened where, and why.
[16:07] scoates:
yeah, the type hint keeps the stack at least one layer thinner (-:
[16:08] _nate_:
Particularly in consideration of the fact that this affects almost every class in the framework, this is a huge improvement. :-)
[16:09] gwoo:
_nate_: something for 0.7?
[16:10] _nate_:
gwoo: Yeah, need to rewrite the signature for every constructor, and every method that takes a $config or $options parameter.
[16:10] gwoo:
yeah
[16:11] scoates:
on a related note...
[16:11] scoates:
this might be a perfect way to do that: http://github.com/facebook/lex-pass
[16:11] jperras:
scoates: isn't type hinting pretty expensive ?
[16:12] jperras:
I thought the lexer had to do a second pass for hinting.
[16:12] scoates:
the lexer? who cares?
[16:12] scoates:
the lexer's performance shouldn't hurt anyone running an opcode cache
[16:12] _nate_:
QotD ^^
[16:12] jperras:
true
[16:13] scoates:
but IMO, this isn't a an optimization, this is a development aid (-:
[16:13] jperras:
I think it was on the internals list a while back
[16:13] scoates:
I probably should read that once in a while
[16:13] _nate_:
As far as I'm concerned it's both, since it allows us to drop a bunch of type casts all over the place, too.
[16:13] scoates:
I used to read it religiously, but I rant out of patience for pettiness and netholes.
[16:14] scoates:
jperras: I'm not up to date on the internals, but as of 5.3, PHP does typechecking on zend_parse_parameters() now, anyway
[16:15] _nate_:
scoates: Can you open a ticket on that sanity check in create() ? I'm juggling a bunch of other things and I already know I'm gonna forget about it.
}}}
Updates
on 02.18.10
by nate
- version was changed to lithium-0.7
- owner was changed to nate
- status was changed to in progress
(fixed)
on 04.19.10
by nate
- status was changed to closed
- resolution was changed to fixed