Re: Routing and controller arguments in K3
Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> Mon, 15 Apr 2013 23:44:36 +0200
| Newsgroups | gmane.comp.web.gallery.devel |
|---|---|
| Message-ID | <CA+z51A6Q_wzC6K60wV6pu1VPgp6FL7OWin1h4BVinRK9-sN90A@mail.gmail.com> |
--===============0770732585580679082== Content-Type: multipart/alternative; boundary=089e012281b4bd52da04da6d2887 --089e012281b4bd52da04da6d2887 Content-Type: text/plain; charset=UTF-8 (er, unfound -> not found. Maybe me talk pretty one day...) Sent from my Swiss Army Phone On Apr 15, 2013 11:30 PM, "Shad Laws" <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote: > 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 > --089e012281b4bd52da04da6d2887 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable <p>(er, unfound -> not found. Maybe me talk pretty one day...)</p> <p>Sent from my Swiss Army Phone</p> <div class=3D"gmail_quote">On Apr 15, 2013 11:30 PM, "Shad Laws" = <<a href=3D"mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]">shad-xpYdmXCiSuZWk0Htik3J/[email protected]</a>> wrote:<b= r type=3D"attribution"><blockquote class=3D"gmail_quote" style=3D"margin:0 = 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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> </blockquote></div> --089e012281b4bd52da04da6d2887-- --===============0770732585580679082== 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 --===============0770732585580679082== 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 ] --===============0770732585580679082==--