Re: Capitalization

Hey Bharat,

Re: the admin controller, more-or-less yes, but with one typo correction:
  modules/gallery/classes/Controller/Admin/AdvancedSettings.php
extends:
  modules/gallery/classes/Gallery/Controller/Admin/AdvancedSettings.php

---

Re: the naming of the gallery-imposed helpers, I like the idea of
flattening the hierarchy a bit and using the now-unique filenames as a
reason why we don't need the "Module" name stuck in there.  However,
in sync with K3-style, I think it'd be better to be "bigendian-like"
with our filenames and use prefixes rather than suffixes to specify
type.  Here's my slightly-changed counter-proposal:
  class [Module]_[Type][Method] {
    // do something
  }
then transparently extend it with:
  class [Type][Module] extends [Module]_[Type][Module] {}

So:
  modules/tag/classes/Tag/EventTag.php
has:
  class Tag_EventTag { // cut'n'paste from current tag_event.php }
and is extended by:
  modules/tag/classes/EventTag.php
which has:
  class EventTag extends Tag_EventTag {}

---

Re: to overload or not to overload, that is the question.  Hamlet
aside, I've spent a decent amount of time now acquainting myself with
this stuff and staring at one-too-many K2 and K3 directory trees, and
my feeling is this: allow overloading of 100% of classes.

My reason is this: the list of "rules" for module devs to follow is
short and has zero exceptions.  In fact, I'd argue that it's easier
than K2 and it's MY_, _Core, _Driver, uppercase
controller/library/model and lowercase helper, etc.  It's a bummer
that we double the file count, but the extra files can be 100%
systematically generated... and in fact I'm already working on a
script to do it for us.

My first sketch of "the rules":

1. Put all "real" code in modules/[module]/classes/[Module].  All file
and directory names should be capitalized and have no underscores.

2. The class name is a direct "/" => "_" search-and-replace job with
the path in classes (e.g.
modules/tag/classes/Gallery/Controllers/Albums.php is
Gallery_Controllers_Albums).

3. If you want to transparently extend something (K2's MY_
functionality), do it directly with the name of the other "real" class
from the base module (e.g. modules/gallery/classes/Gallery/View.php
has "class Gallery_View extends Kohana_View").

4. If you want to extend something with a new name, do it with the
name of the other already-extended class (e.g.
modules/gallery/classes/Gallery/Controller/Albums.php has "class
Gallery_Controller_Albums extends Controller_Items").

5. For every file you have in modules/[module]/classes/[Module], make
one of the same name and path in modules/[module]/classes that has
"class [Something] extends [Module]_[Something]".  Or, run the script
I'm working on to do it for you.

This makes it easy to know that 100% of "real" code is in the [Module]
subdirectory.  If we only allow some stuff to be overloaded and not
others, we break this.  Based on my (limited) experience, this
actually makes it harder to browse code.  It'd also make it harder to
auto-generate (and build unit tests for, of course) the
transparent-extension-enabling files.

---

I shared with you a google spreadsheet of the 71 K3-converted classes
we have so far.  My goal is to keep growing the list, and then use it
as the starting point for an app-wide grep to do most of the
replacement for us.  If anyone else would like to take a peek, let me
know and I'm happy to share it!

Take care,
Shad



On 18 March 2013 17:02, Bharat Mediratta <[email protected]> wrote:
>
> Thanks for the detailed summary.
>
>
> On Mon, Mar 18, 2013 at 8:33 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>>
>> Translating the controller class pattern is fairly obvious: it should
>> be [Module]_Controller_Admin_[Something], which is then transparently
>> extended as Controller_Admin_[Something].  This flows nicely with K3.
>
>
> Ok, so Albums_Controller becomes:
>   modules/gallery/classes/Controller/Albums.php
>
> which extends:
>   modules/gallery/classes/Gallery/Controller/Albums.php
>
> and:
>   modules/gallery/classes/Controller/Admin/AdvancedSettings.php
>
> extends:
>   modules/gallery/classes/Controller/Gallery/Admin/AdvancedSettings.php
>
> That makes sense.
>
>>
>> Translating the helper class patterns is slightly less obvious.  Here
>> are some possibilities:
>>
>>
>> 1. Call them [Module]_Module_[Type], e.g. tag_event_Core =>
>> Tag_Module_Event.  The asterisk to this approach is that, unlike every
>> other class in the classes directory, these should *not* get the
>> transparent-extension-enabling class added to it (e.g. "class
>> Module_Event extends Tag_Module_Event {}").
>>
>>
>> 2. Call them [Module]_[Module]_[Type], e.g. tag_event_Core =>
>> Tag_Tag_Event.  This is similar to the previous approach, but keeps
>> the transparent-extension-enabling class added to it (e.g. "class
>> Tag_Event extends Tag_Tag_Event {}").  This means that, in the end,
>> it's called by other functions as Tag_Event.  The asterisk to this
>> approach is that the directory structure is a little broken.  In this
>> example, the tag module would have all of its "real" class code in
>> modules/tag/classes/Tag, which are transparently extended by files in
>> the parent of that directory.  So, the event code would be in
>> /modules/tag/classes/Tag/Tag/Event.php, and the
>> transparent-extension-enabling file at
>> /modules/tag/classes/Tag/Event.php, with all of its siblings being
>> "real" classes instead of the thin extensions.  It works, but it
>> doesn't seem clean.
>>
>> 3. Call them Module_[Module]_[Type], e.g. tag_event_Core =>
>> Module_Tag_Event.  This has the same problem as 2, in an even
>> less-idiomatic way.
>>
>> 4. Call them [Module]_[Type]_[Module], e.g. tag_event_Core =>
>> Tag_Event_Tag, which is then transparently extended as Event_Tag.
>> This doesn't seem like a great idea since examples like Rest_Gallery
>> from the gallery module seem likely to clobber something in the rest
>> module.
>>
>> 5. Call them [Module]_Module_[Type]_[Module], e.g. tag_event_Core =>
>> Tag_Module_Event_Tag, which is then transparently extended as
>> Module_Event_Tag.  This flows pretty well with K3, and is my current
>> favorite approach.
>>
>> 6. Put them somewhere else and bypass the standard autoloader.  This
>> has the advantage of allowing us to make the class names look less
>> encumbered (e.g. tag_event_Core => tag_event), but otherwise seems to
>> intentionally break all of the other Kohana conventions.
>
>
> 7.  Go with [Module][Type], eg tag_event_Core => TagEvent, which is then
> transparently extended as Tag_TagEvent.  This flattens out one level of the
> hierarchy and gets rid of the hierarchy altogether for modules that don't
> want to make extending easy.  It has the downside of making it harder to
> figure out which things are part of the Gallery-imposed infrastructure of a
> module (ie - grouping the _task, _installer, etc stuff together).
>
> I think we want to do our best to avoid requiring module developers to
> understand a lot of structure.  K2 was great about that - K3 is making it
> harder with all this hierarchical complexity.  I get why they did it, but
> frankly the overloading mechanism was a lot less boilerplate in K2.  Now
> we're basically going to have to double our file size (cry!) to make
> everything overloadable.  That really sucks.
>
> Seeing where this is going - I think that we should do two things here:
>
> 1) We should not blindly make everything overloadable.  99% (pulling that
> number out of nowhere) of what we make overloadable will never be overloaded
> and just adds extra cognitive pain to the app.
>
> 2) We should keep directory structures flat where possible.
> modules/tag/helpers/tag_event should become a non overloadable
> modules/tag/classes/TagEvent.  If at some point there's a strong desire to
> start overloading this stuff we should look really closely at why they want
> to overload and see if there's some better mechanism.  Overloading is likely
> an indication that we're doing something else wrong.
>
> thoughts?

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
__[ 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 ]