Re: Routing and controller arguments in K3
Bharat Mediratta <[email protected]> Mon, 15 Apr 2013 10:44:12 -0700
| Newsgroups | gmane.comp.web.gallery.devel |
|---|---|
| Message-ID | <CAESa+_mHgOH=a2AOen2j_9oh8qBwSBkcJGWGKO=HsKeLV=eKWw@mail.gmail.com> |
--===============3039615463473293335== Content-Type: multipart/alternative; boundary=047d7bdca35264541d04da69cf4e --047d7bdca35264541d04da69cf4e Content-Type: text/plain; charset=ISO-8859-1 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. On Mon, Apr 15, 2013 at 9:18 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote: > Hey Bharat, > > 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? [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? [snip] > It seems like the biggest downside to this approach is that, if you want a > reverse route, you need to remember the argument order (e.g. "type" goes > before "theme_name" in "args"). Furthermore, changing the argument order > breaks the reverse routes. But, the alternative is configuring the routes > ahead of time, adding an extra place to define things... which I, > personally, feel worse about. > Agreed, though we're in no worse a situation than we were before - so if we go this route we can always change our minds later and try switching over to named params. -Bharat --047d7bdca35264541d04da69cf4e Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><br><div style>On the one hand, it's kind of awesome t= o define an exact route for each controller. =A0But the overhead of doing t= hat would kind of suck - it would mean that every action in every controlle= r would have to have its own regexp. =A0I can't help but think that thi= s will be slow. =A0And cumbersome from a maintenance perspective. =A0So you= 've convinced me - I'm grudgingly letting go of the "named par= ameters" idea. =A0</div> <div style><br></div><div class=3D"gmail_extra"><div class=3D"gmail_quote">= On Mon, Apr 15, 2013 at 9:18 AM, Shad Laws <span dir=3D"ltr"><<a href=3D= "mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]" target=3D"_blank">shad-xpYdmXCiSuZWk0Htik3J/[email protected]</a>></spa= n> wrote:<br> <blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p= x #ccc solid;padding-left:1ex">Hey Bharat,<div><br></div><div>The good news= is that the whole isset/default capability is already baked into K3's = Request::param(). =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>=A0=A0</div><div style>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. =A0Take Controller_Quick::action_form_delete($id) as = an example. =A0In K2, if you don't specify an id, it errors before we e= ven get into the function because of a missing parameter. =A0In K3 we'd= have to check to see if the parameter exists and then throw an exception i= f it's missing. =A0We'll be doing that all over the place, right?</= div> <div style><br></div><div style>Granted, in many places we're dealing w= ith an $id which we turn into a Model_Item and then call Access::required o= n it so if the $id is missing we're ok - but for this to work we have t= o be clear that a required parameter can always be null. =A0I haven't a= udited the code exhaustively to see if there are any places where that'= s going to really bite us. =A0You've spent a little more time recently = looking at our controllers - what do you think?</div> <div style><br></div><div style>[snip]</div><blockquote class=3D"gmail_quot= e" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">= <div><div><span style=3D"font-family:'courier new',monospace">=A0 p= ublic function args($index, $default=3Dnull) {</span></div> <div><font face=3D"courier new, monospace">=A0 =A0 if ($this->_args =3D= =3D=3D null) {</font></div> <div><span style=3D"font-family:'courier new',monospace">=A0 =A0 = =A0 $this->_args =3D trim($this->request->param("args"),= "/");</span></div><div><span style=3D"font-family:'courier n= ew',monospace">=A0 =A0 =A0 $this->_args =3D explode("/",= =A0</span><span style=3D"font-family:'courier new',monospace">(arra= y)=A0</span><span style=3D"font-family:'courier new',monospace">$th= is->_args</span><span style=3D"font-family:'courier new',monospa= ce">);</span></div> <div><font face=3D"courier new, monospace">=A0 =A0 =A0 $this->_args =3D= =A0</font><span style=3D"font-family:'courier new',monospace">Purif= ier::clean_html(</span><span style=3D"font-family:'courier new',mon= ospace">$this->_args</span><span style=3D"font-family:'courier new&#= 39;,monospace">);</span></div> <div><font face=3D"courier new, monospace">=A0 =A0 }</font></div><div><font= face=3D"courier new, monospace">=A0 =A0 return Arr::get($this->_args, $= index, $default);</font></div><div><font face=3D"courier new, monospace">= =A0 }</font></div> </div></blockquote><div><br></div><div style>I like this. =A0Some tiny nits= - usually we use isset() instead of "=3D=3D=3D null". =A0Also, I= think "arg" is a better API than "args" because we'= ;re only ever returning a single arg, right?</div> <div><br></div><div style>[snip]=A0</div><blockquote class=3D"gmail_quote" = style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><di= v>It seems like the biggest downside to this approach is that, if you want = a reverse route, you need to remember the argument order (e.g. "type&q= uot; goes before "theme_name" in "args"). =A0Furthermor= e, changing the argument order breaks the reverse routes. =A0But, the alter= native is configuring the routes ahead of time, adding an extra place to de= fine things... which I, personally, feel worse about.</div> </blockquote><div><br></div><div style>Agreed, though we're in no worse= a situation than we were before - so if we go this route we can always cha= nge our minds later and try switching over to named params.</div><div style= > <br></div><div style>-Bharat</div><div>=A0</div></div></div></div> --047d7bdca35264541d04da69cf4e-- --===============3039615463473293335== 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 --===============3039615463473293335== 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 ] --===============3039615463473293335==--