Re: Routing and controller arguments in K3
Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> Mon, 15 Apr 2013 23:30:32 +0200
| Newsgroups | gmane.comp.web.gallery.devel |
|---|---|
| Message-ID | <CA+z51A7OkwTgc9b1nsQ7+L0-s3wna7UWVh5fv7Te6QrVYBrcYw@mail.gmail.com> |
--===============3304511901531425052==
Content-Type: multipart/alternative; boundary=089e0115f20e9a423a04da6cf7f0
--089e0115f20e9a423a04da6cf7f0
Content-Type: text/plain; charset=UTF-8
Hey Bharat,
On the one hand, it's kind of awesome to define an exact route for each
> controller. But the overhead of doing that would kind of suck - it would
> mean that every action in every controller would have to have its own
> regexp. I can't help but think that this will be slow. And cumbersome
> from a maintenance perspective. So you've convinced me - I'm grudgingly
> letting go of the "named parameters" idea.
>
>
Aye - it seems like the better of two options. It's not without it's own
shortcomings, which leaves us both a bit begrudged, but it seems like the
best way to go.
> The good news is that the whole isset/default capability is already baked
>> into K3's Request::param(). So:
>> $this->request->param("undefined_foo") --> null
>> $this->request->param("undefined_foo", "default") --> "default"
>> $this->request->param("defined_foo") --> "bar"
>>
>
> Ok, but my fear here is that if you don't define a mandatory parameter,
> it's not clear that there's always a fallback. Take
> Controller_Quick::action_form_delete($id) as an example. In K2, if you
> don't specify an id, it errors before we even get into the function because
> of a missing parameter. In K3 we'd have to check to see if the parameter
> exists and then throw an exception if it's missing. We'll be doing that
> all over the place, right?
>
> Granted, in many places we're dealing with an $id which we turn into a
> Model_Item and then call Access::required on it so if the $id is missing
> we're ok - but for this to work we have to be clear that a required
> parameter can always be null. I haven't audited the code exhaustively to
> see if there are any places where that's going to really bite us. You've
> spent a little more time recently looking at our controllers - what do you
> think?
>
Hmm, good point. It's clear that there's going to be *some* error
*somewhere*. The question is just where, and whether or not we control
what it is. Since it's URL-based, perhaps a 404 exception makes sense?
> [snip]
>
>> public function args($index, $default=null) {
>> if ($this->_args === null) {
>> $this->_args = trim($this->request->param("args"), "/");
>> $this->_args = explode("/", (array) $this->_args);
>> $this->_args = Purifier::clean_html($this->_args);
>> }
>> return Arr::get($this->_args, $index, $default);
>> }
>>
>
> I like this. Some tiny nits - usually we use isset() instead of "===
> null". Also, I think "arg" is a better API than "args" because we're only
> ever returning a single arg, right?
>
Re: isset(), sure, can do.
Re: arg vs. args, sure, makes sense. It's also consistent with the name
param().
Re: single vs. multiple, I'm starting to think it might make sense to allow
the return of the complete array, too. Similar to the previous point, it
has the added benefit of making us consistent with param(), which works
with $_params.
Here's take two. No arguments means give full array; one argument means
required; two arguments means not required with default value specified.
public function arg($index=null, $default=null) {
// Initialize the array if needed.
if (!isset($this->_args)) {
$this->_args = trim($this->request->param("args"), "/");
$this->_args = explode("/", (array) $this->_args);
$this->_args = Purifier::clean_html($this->_args);
}
// If required and unfound, throw 404 exception.
if ((func_num_args() == 1) && !isset($this->_args[$index])) {
throw HTTP_Exception::factory(404);
}
return isset($index) ? Arr::get($this->_args, $index, $default) :
$this->_args;
}
Thoughts?
Take care,
Shad
--089e0115f20e9a423a04da6cf7f0
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Hey Bharat,<div><br></div><div><br><div class=3D"gmail_quote"><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;=
padding-left:1ex"><div dir=3D"ltr"><div>On the one hand, it's kind of a=
wesome to define an exact route for each controller. =C2=A0But the overhead=
of doing that would kind of suck - it would mean that every action in ever=
y controller would have to have its own regexp. =C2=A0I can't help but =
think that this will be slow. =C2=A0And cumbersome from a maintenance persp=
ective. =C2=A0So you've convinced me - I'm grudgingly letting go of=
the "named parameters" idea. =C2=A0</div>
<div><br></div></div></blockquote><div><br></div><div>Aye - it seems like t=
he better of two options. =C2=A0It's not without it's own shortcomi=
ngs, which leaves us both a bit begrudged, but it seems like the best way t=
o go.</div>
<div><br></div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"=
margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir=3D"=
ltr"><div></div><div class=3D"gmail_extra"><div class=3D"gmail_quote"><bloc=
kquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #cc=
c solid;padding-left:1ex">
<div>The good news is that the whole isset/default capability is already ba=
ked into K3's Request::param(). =C2=A0So:</div>
<div>$this->request->param("undefined_foo") --> null</di=
v>
<div><div>$this->request->param("undefined_foo", "defa=
ult") --> "default"</div><div><div>$this->request->=
param("defined_foo") --> "bar"</div></div></div>
</blockquote><div>=C2=A0=C2=A0</div><div>Ok, but my fear here is that if yo=
u don't define a mandatory parameter, it's not clear that there'=
;s always a fallback. =C2=A0Take Controller_Quick::action_form_delete($id) =
as an example. =C2=A0In K2, if you don't specify an id, it errors befor=
e we even get into the function because of a missing parameter. =C2=A0In K3=
we'd have to check to see if the parameter exists and then throw an ex=
ception if it's missing. =C2=A0We'll be doing that all over the pla=
ce, right?</div>
</div></div></div></blockquote><blockquote class=3D"gmail_quote" style=3D"m=
argin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir=3D"l=
tr"><div class=3D"gmail_extra"><div class=3D"gmail_quote"><div><br></div><d=
iv>Granted, in many places we're dealing with an $id which we turn into=
a Model_Item and then call Access::required on it so if the $id is missing=
we're ok - but for this to work we have to be clear that a required pa=
rameter can always be null. =C2=A0I haven't audited the code exhaustive=
ly to see if there are any places where that's going to really bite us.=
=C2=A0You've spent a little more time recently looking at our controll=
ers - what do you think?</div>
</div></div></div></blockquote><div><br></div><div><div>Hmm, good point. =
=C2=A0It's clear that there's going to be *some* error *somewhere*.=
=C2=A0The question is just where, and whether or not we control what it is=
. =C2=A0Since it's URL-based, perhaps a 404 exception makes sense?</div=
>
</div><div><br></div><div>=C2=A0</div><blockquote class=3D"gmail_quote" sty=
le=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div d=
ir=3D"ltr"><div class=3D"gmail_extra"><div class=3D"gmail_quote"><div></div=
><div>[snip]</div>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex"><div><div><span style=3D"font-family:'co=
urier new',monospace">=C2=A0 public function args($index, $default=3Dnu=
ll) {</span></div>
<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 if ($this->_arg=
s =3D=3D=3D null) {</font></div>
<div><span style=3D"font-family:'courier new',monospace">=C2=A0 =C2=
=A0 =C2=A0 $this->_args =3D trim($this->request->param("args&=
quot;), "/");</span></div><div><span style=3D"font-family:'co=
urier new',monospace">=C2=A0 =C2=A0 =C2=A0 $this->_args =3D explode(=
"/",=C2=A0</span><span style=3D"font-family:'courier new'=
,monospace">(array)=C2=A0</span><span style=3D"font-family:'courier new=
',monospace">$this->_args</span><span style=3D"font-family:'cour=
ier new',monospace">);</span></div>
<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 =C2=A0 $this->_=
args =3D=C2=A0</font><span style=3D"font-family:'courier new',monos=
pace">Purifier::clean_html(</span><span style=3D"font-family:'courier n=
ew',monospace">$this->_args</span><span style=3D"font-family:'co=
urier new',monospace">);</span></div>
<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 }</font></div><div=
><font face=3D"courier new, monospace">=C2=A0 =C2=A0 return Arr::get($this-=
>_args, $index, $default);</font></div><div><font face=3D"courier new, m=
onospace">=C2=A0 }</font></div>
</div></blockquote><div><br></div><div>I like this. =C2=A0Some tiny nits - =
usually we use isset() instead of "=3D=3D=3D null". =C2=A0Also, I=
think "arg" is a better API than "args" because we'=
;re only ever returning a single arg, right?</div>
</div></div></div></blockquote><div><br></div><div>Re: isset(), sure, can d=
o.</div><div><br></div><div>Re: arg vs. args, sure, makes sense. =C2=A0It&#=
39;s also consistent with the name param().</div><div><br></div><div>Re: si=
ngle vs. multiple, I'm starting to think it might make sense to allow t=
he return of the complete array, too. =C2=A0Similar to the previous point, =
it has the added benefit of making us consistent with param(), which works =
with $_params.</div>
<div><br></div><div>Here's take two. =C2=A0No arguments means give full=
array; one argument means required; two arguments means not required with =
default value specified.</div><div><br></div><div><div><span style=3D"font-=
family:'courier new',monospace">=C2=A0 public function arg($index=
=3Dnull, $default=3Dnull) {</span></div>
<div><span style=3D"font-family:'courier new',monospace">=C2=A0 =C2=
=A0 // Initialize the array if needed.=C2=A0</span></div><div><font face=3D=
"courier new, monospace">=C2=A0 =C2=A0 if (!isset($this->_args)) {</font=
></div><div><span style=3D"font-family:'courier new',monospace">=C2=
=A0 =C2=A0 =C2=A0 $this->_args =3D trim($this->request->param(&quo=
t;args"), "/");</span></div>
<div><span style=3D"font-family:'courier new',monospace">=C2=A0 =C2=
=A0 =C2=A0 $this->_args =3D explode("/",=C2=A0</span><span sty=
le=3D"font-family:'courier new',monospace">(array)=C2=A0</span><spa=
n style=3D"font-family:'courier new',monospace">$this->_args</sp=
an><span style=3D"font-family:'courier new',monospace">);</span></d=
iv>
<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 =C2=A0 $this->_=
args =3D=C2=A0</font><span style=3D"font-family:'courier new',monos=
pace">Purifier::clean_html(</span><span style=3D"font-family:'courier n=
ew',monospace">$this->_args</span><span style=3D"font-family:'co=
urier new',monospace">);</span></div>
<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 }</font></div><div=
><font face=3D"courier new, monospace"><br></font></div><div><font face=3D"=
courier new, monospace">=C2=A0 =C2=A0 // If required and unfound, throw 404=
exception.</font></div>
<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 if ((func_num_args=
() =3D=3D 1) && !isset($this->_args[$index])) {</font></div><div=
><span style=3D"font-family:'courier new',monospace">=C2=A0 =C2=A0 =
=C2=A0 throw HTTP_Exception::factory(404);</span></div>
<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 }</font></div><div=
><font face=3D"courier new, monospace"><br></font></div><div><span style=3D=
"font-family:'courier new',monospace">=C2=A0 =C2=A0 return isset($i=
ndex) ? Arr::get($this->_args, $index, $default) : $this->_args;</spa=
n></div>
<div><font face=3D"courier new, monospace">=C2=A0 }</font></div></div><div>=
<br></div><div>Thoughts?</div><div><br></div><div>Take care,</div><div>Shad=
</div></div></div>
--089e0115f20e9a423a04da6cf7f0--
--===============3304511901531425052==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
--===============3304511901531425052==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
__[ g a l l e r y - d e v e l ]_________________________
[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]
--===============3304511901531425052==--