Re: Routing and controller arguments in K3

Bharat Mediratta <[email protected]> Sat, 13 Apr 2013 22:15:52 -0700
Newsgroups gmane.comp.web.gallery.devel
Message-ID <CAESa+_mMX9NucivYWgaq_QNvorAZhGu+LW-Jd-9+R506Gtt8cg@mail.gmail.com>
--===============3046679757959207007==
Content-Type: multipart/alternative; boundary=20cf3005dc544a2e7804da4b3db5

--20cf3005dc544a2e7804da4b3db5
Content-Type: text/plain; charset=ISO-8859-1

Finally getting around to this!  I'm sad to see the controller action
function args go away - they were very convenient.  I wonder why they were
removed?

Either way - I think that $this->request->param("...") is pretty unwieldy.
 I was looking to see whether we should reinstate this functionality and I
found http://dev.kohanaframework.org/issues/3536 which tells the back story.

In essence, they're saying that generic arguments are a bad idea because
they break reverse routing.  Instead, every argument should be a named
argument.  This also allows for reverse routing, which I think will be
essential for our embedding story.  So for the most part, this is easy
because we're generally dealing with $id as our argument.  But it means
that we shouldn't use $args as a generic arg holder, but we should stick
with names.

So Admin_Themes in K2 has:

  function preview($type, $theme_name) {
    ...
  }

and that would become this in K3:

  function action_preview() {
    $type = $this->request->param("type");
    $theme_name = $this->request->param($theme_name);
    ...
  }

One interesting thing to note here is that in K2, both of those args are
required so if you leave off one of them it throws an error.  In K3, I
don't see a facility for required params - have you seen anything about
that?  Without that, we'll need to check for missing params all the time..
ugh.  We need a shorthand for this *and* we need an easy way to define a
param as mandatory.

Proposal:
Create a base class controller function that provides two members: param
and optional_param. The API would look like this:

  function action_preview() {
    $type = $this->param->type;   *// throw exception if type is not defined
*
    $theme_name = $this->param->theme_name;
    $some_value = $this->optional_param->some_value;
      *// return null if some_value is not defined*
    ...
  }

The code would look like this:

  class Controller_Params {
    public function __construct($params, $mandatory=true) {
      $this->_params = $params;
    }

    function __get($key) {
      if (!isset($this->_params[$key])) {
        if ($mandatory) {
          throw new Exception("Missing argument for $key");
        }
        return null;
      }
      return $this->_params[$key];
    }
  }

Then in the Gallery_Controller base class (or the default before() or
whatever we do across all controllers):

  public function __construct() {
    $this->params = new Controller_Params($this->request->params);
    $this->optional_params = new Controller_Params($this->request->params);
  }

This is just a straw man.  Please shred it :-)


On Wed, Apr 10, 2013 at 3:32 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:

> Hey gang,
>
> So, calling controller actions with arguments was carried over to 3.0,
> deprecated in 3.1, and removed in 3.2.
>
> Example: we want index.php/bar/foo/123.  What used to be this:
>
> public function foo($id=null) {
>   // Do something with $id = 123...
> }
>
> Will now need to be something like this:
>
> public function action_foo() {
>   $id = $this->request->param("id", null);
>   // Do something with $id = 123...
> }
>
> With a route defined like this:
>
> Route::set("example", "(<controller>(/<action>(/<id>)))")
>
> A quick survey of Gallery's controllers (in the core repo) shows that,
> in 162 controller actions, we have:
> - 81 with 0 args
> - 70 with 1 arg (typically named something like $id)
> - 9 with 2 args
> - 1 with 3 args
> - 1 with 4 args
> - 0 with 5+ args
>
> So, here's my proposal: we optimize ourselves for 0-1 arguments, and
> make a flexible system to handle 2+.  We use a route something like
> this:
>
> Route::set("example", "(<controller>(/<action>(/<id>(/<args>))))")
>   ->filter( // Use an explode for "/" to parse args into an array here )
>
> And then access them like this:
>
> $id = $this->request->param("id");
> $args = $this->request->param("args");
> $second = $args[0];
> $third = $args[1];
> $fourth = $args[2];
> $fifth = $args[3];
> ...
> $fiftieth = $arg[48];
>
> This keeps our typical cases simple and lean, while being totally
> flexible for our other 11 cases as well as anything a contrib module
> could dream up.  Does this seem like a reasonable approach?
>
> Take care,
> Shad
>
>
> ------------------------------------------------------------------------------
> 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
> __[ 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 ]
>
>

--20cf3005dc544a2e7804da4b3db5
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><br><div style>Finally getting around to this! =A0I&#39;m =
sad to see the controller action function args go away - they were very con=
venient. =A0I wonder why they were removed?</div><div style><br></div><div =
style>

Either way - I think that $this-&gt;request-&gt;param(&quot;...&quot;) is p=
retty unwieldy. =A0I was looking to see whether we should reinstate this fu=
nctionality and I found=A0<a href=3D"http://dev.kohanaframework.org/issues/=
3536">http://dev.kohanaframework.org/issues/3536</a> which tells the back s=
tory.</div>

<div style><br></div><div style>In essence, they&#39;re saying that generic=
 arguments are a bad idea because they break reverse routing. =A0Instead, e=
very argument should be a named argument. =A0This also allows for reverse r=
outing, which I think will be essential for our embedding story. =A0So for =
the most part, this is easy because we&#39;re generally dealing with $id as=
 our argument. =A0But it means that we shouldn&#39;t use $args as a generic=
 arg holder, but we should stick with names.</div>

<div style><br></div><div style>So Admin_Themes in K2 has:</div><div style>=
<br></div><div style><font face=3D"courier new, monospace">=A0 function pre=
view($type, $theme_name) {</font></div><div style><font face=3D"courier new=
, monospace">=A0 =A0 ...</font></div>

<div style><font face=3D"courier new, monospace">=A0 }</font></div><div sty=
le><br></div><div style>and that would become this in K3:</div><div style><=
br></div><div style><font face=3D"courier new, monospace">=A0 function acti=
on_preview() {</font></div>

<div style><font face=3D"courier new, monospace">=A0 =A0 $type =3D $this-&g=
t;request-&gt;param(&quot;type&quot;);</font></div><div style><font face=3D=
"courier new, monospace">=A0 =A0 $theme_name =3D $this-&gt;request-&gt;para=
m($theme_name);</font></div>

<div style><font face=3D"courier new, monospace">=A0 =A0 ...</font></div><d=
iv style><font face=3D"courier new, monospace">=A0 }</font></div><div style=
><br></div><div style>One interesting thing to note here is that in K2, bot=
h of those args are required so if you leave off one of them it throws an e=
rror. =A0In K3, I don&#39;t see a facility for required params - have you s=
een anything about that? =A0Without that, we&#39;ll need to check for missi=
ng params all the time.. ugh. =A0We need a shorthand for this <b>and</b>=A0=
we need an easy way to define a param as mandatory.</div>

<div style><br></div><div style>Proposal:</div><div style>Create a base cla=
ss controller function that provides two members: param and optional_param.=
 The API would look like this:</div><div style><br></div><div style><div>

<font face=3D"courier new, monospace">=A0 function action_preview() {</font=
></div><div><font face=3D"courier new, monospace">=A0 =A0 $type =3D $this-&=
gt;param-&gt;type; =A0 <b>// throw exception if <i>type</i>=A0is not define=
d</b></font></div>

<div><font face=3D"courier new, monospace">=A0 =A0 $theme_name =3D $this-&g=
t;param-&gt;theme_name;</font></div><div><div><font face=3D"courier new, mo=
nospace">=A0 =A0 $some_value =3D $this-&gt;optional_param-&gt;some_value;</=
font></div>
<div>
<font face=3D"courier new, monospace">=A0 =A0 =A0=A0<b>// return null if so=
me_value is not defined</b></font></div></div><div><font face=3D"courier ne=
w, monospace">=A0 =A0 ...</font></div><div><font face=3D"courier new, monos=
pace">=A0 }</font></div>

<div><br></div><div style>The code would look like this:</div><div style><b=
r></div><div style><font face=3D"courier new, monospace">=A0 class Controll=
er_Params {</font></div><div style><font face=3D"courier new, monospace">=
=A0 =A0 public function __construct($params, $mandatory=3Dtrue) {</font></d=
iv>

<div style><font face=3D"courier new, monospace">=A0 =A0 =A0 $this-&gt;_par=
ams =3D $params;</font></div><div style><font face=3D"courier new, monospac=
e">=A0 =A0 }</font></div><div style><font face=3D"courier new, monospace"><=
br></font></div>

<div style><font face=3D"courier new, monospace">=A0 =A0 function __get($ke=
y) {</font></div><div style><font face=3D"courier new, monospace">=A0 =A0 =
=A0 if (!isset($this-&gt;_params[$key])) {</font></div><div style><font fac=
e=3D"courier new, monospace">=A0 =A0 =A0 =A0 if ($mandatory) {</font></div>

<div style><font face=3D"courier new, monospace">=A0 =A0 =A0 =A0 =A0 throw =
new Exception(&quot;Missing argument for $key&quot;);</font></div><div styl=
e><font face=3D"courier new, monospace">=A0 =A0 =A0 =A0 }</font></div><div =
style><font face=3D"courier new, monospace">=A0 =A0 =A0 =A0 return null;</f=
ont></div>

<div style><font face=3D"courier new, monospace">=A0 =A0 =A0 }</font></div>=
<div style><font face=3D"courier new, monospace">=A0 =A0 =A0 return $this-&=
gt;_params[$key];</font></div><div style><font face=3D"courier new, monospa=
ce">=A0 =A0 }</font></div>

<div style><font face=3D"courier new, monospace">=A0 }</font></div><div sty=
le><br></div><div style>Then in the Gallery_Controller base class (or the d=
efault before() or whatever we do across all controllers):</div><div style>

<br></div><div style><font face=3D"courier new, monospace">=A0 public funct=
ion __construct() {</font></div><div style><font face=3D"courier new, monos=
pace">=A0 =A0 $this-&gt;params =3D new Controller_Params($this-&gt;request-=
&gt;params);</font></div>

<div style><div><font face=3D"courier new, monospace">=A0 =A0 $this-&gt;opt=
ional_params =3D new Controller_Params($this-&gt;request-&gt;params);</font=
></div><div><font face=3D"courier new, monospace">=A0 }</font></div><div><f=
ont face=3D"courier new, monospace"><br>

</font></div><div style><font face=3D"arial, helvetica, sans-serif">This is=
 just a straw man. =A0Please shred it :-)</font></div></div></div></div><di=
v class=3D"gmail_extra"><br><br><div class=3D"gmail_quote">On Wed, Apr 10, =
2013 at 3:32 PM, Shad Laws <span dir=3D"ltr">&lt;<a href=3D"mailto:shad@sha=
dlaws.com" target=3D"_blank">shad-xpYdmXCiSuZWk0Htik3J/[email protected]</a>&gt;</span> wrote:<br>

<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">Hey gang,<br>
<br>
So, calling controller actions with arguments was carried over to 3.0,<br>
deprecated in 3.1, and removed in 3.2.<br>
<br>
Example: we want index.php/bar/foo/123. =A0What used to be this:<br>
<br>
public function foo($id=3Dnull) {<br>
=A0 // Do something with $id =3D 123...<br>
}<br>
<br>
Will now need to be something like this:<br>
<br>
public function action_foo() {<br>
=A0 $id =3D $this-&gt;request-&gt;param(&quot;id&quot;, null);<br>
=A0 // Do something with $id =3D 123...<br>
}<br>
<br>
With a route defined like this:<br>
<br>
Route::set(&quot;example&quot;, &quot;(&lt;controller&gt;(/&lt;action&gt;(/=
&lt;id&gt;)))&quot;)<br>
<br>
A quick survey of Gallery&#39;s controllers (in the core repo) shows that,<=
br>
in 162 controller actions, we have:<br>
- 81 with 0 args<br>
- 70 with 1 arg (typically named something like $id)<br>
- 9 with 2 args<br>
- 1 with 3 args<br>
- 1 with 4 args<br>
- 0 with 5+ args<br>
<br>
So, here&#39;s my proposal: we optimize ourselves for 0-1 arguments, and<br=
>
make a flexible system to handle 2+. =A0We use a route something like<br>
this:<br>
<br>
Route::set(&quot;example&quot;, &quot;(&lt;controller&gt;(/&lt;action&gt;(/=
&lt;id&gt;(/&lt;args&gt;))))&quot;)<br>
=A0 -&gt;filter( // Use an explode for &quot;/&quot; to parse args into an =
array here )<br>
<br>
And then access them like this:<br>
<br>
$id =3D $this-&gt;request-&gt;param(&quot;id&quot;);<br>
$args =3D $this-&gt;request-&gt;param(&quot;args&quot;);<br>
$second =3D $args[0];<br>
$third =3D $args[1];<br>
$fourth =3D $args[2];<br>
$fifth =3D $args[3];<br>
...<br>
$fiftieth =3D $arg[48];<br>
<br>
This keeps our typical cases simple and lean, while being totally<br>
flexible for our other 11 cases as well as anything a contrib module<br>
could dream up. =A0Does this seem like a reasonable approach?<br>
<br>
Take care,<br>
Shad<br>
<br>
---------------------------------------------------------------------------=
---<br>
Precog is a next-generation analytics platform capable of advanced<br>
analytics on semi-structured data. The platform includes APIs for building<=
br>
apps and a phenomenal toolset for data science. Developers can use<br>
our toolset for easy data analysis &amp; visualization. Get a free account!=
<br>
<a href=3D"http://www2.precog.com/precogplatform/slashdotnewsletter" target=
=3D"_blank">http://www2.precog.com/precogplatform/slashdotnewsletter</a><br=
>
__[ g a l l e r y - d e v e l ]_________________________<br>
<br>
[ list info/archive --&gt; <a href=3D"http://gallery.sf.net/lists.php" targ=
et=3D"_blank">http://gallery.sf.net/lists.php</a> ]<br>
[ gallery info/FAQ/download --&gt; <a href=3D"http://gallery.sf.net" target=
=3D"_blank">http://gallery.sf.net</a> ]<br>
<br>
</blockquote></div><br></div>

--20cf3005dc544a2e7804da4b3db5--


--===============3046679757959207007==
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
--===============3046679757959207007==
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 ]
--===============3046679757959207007==--