Re: Capitalization

Once again, great writeup.  Let me make some counter-points below.

On Mon, Mar 18, 2013 at 11:24 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:

> 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 {}
>

Can you give me an example of the big-endianness in K3?  Even if K3 has a
different style, I'm a big believer that class names should be human
understandable.  "It's an EventTag" makes a lot less sense to me than "it's
a TagEvent".  So I'm still leaning towards:

modules/tag/classes/TagEvent.php:
class TagEvent { ... }

In a perfect world I'd like to even drop the "Tag" bit but since we lack
namespacing, that's not possible.  Came to that same conclusion with K2,
which is why it's "tag_event" there, too :-)

I also think it's too hierarchical which leads me to the next point...


> 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.
>

I agree that K2's approach is sucky.  No argument there.  The interesting
thing about K3 is that they removed a lot of the overloading magic - all
you have to do is understand the order of the cascading filesystem and the
rest you can get by following the code.

But the problem is that most module devs don't *want* to understand this.
 With G3 we're aiming to make it really simple for a dev to write something
useful.  With G3.0.x you can write a useful module with two files, the
module.info and an _event.php file.  Making a class overloadable adds yet
another file to the mix and the file is merely an abstraction point - it
adds zero value to the module dev.  It only adds value to future module
devs.  So from a "I want to get this feature done" perspective it's just
noise.  And the cumulative effect of that noise will be another 70+ files
(guessing based on your other comment) in our codebase whose filenames are
very similar to the files that we *really* care about - the ones that
actually contain the code.

That might still be worthwhile if people actually overloaded stuff that
often - but the truth is that they really don't.

$ cd gallery3-contrib
$ find 3.0/modules/ -name 'MY_*' | awk -F/ '{print $NF}' | sort | uniq -c
     maste
      1 MY_Form_Input.php
      2 MY_Item_Model.php
      1 MY_ORM_MPTT.php
      1 MY_Theme_View.php
      2 MY_access.php
      1 MY_embedlinks_block.php
      1 MY_embedlinks_theme.php
      1 MY_gallery_graphics.php
      6 MY_item.php
      1 MY_search.php
      1 MY_url.php

We don't do that much overloading.  Some of the overloading we do is
probably quite dangerous (ORM_MPTT, Item_Model, access).  It's a bad smell
that we have to do this in the first place.

Finally, the fact that we can systematically generate these helper files
with a script is a sign that we're headed in the wrong direction towards
more boilerplate instead of less.  We want to bridge that line between too
much unnecessary code (boilerplate) and too little (magic).  I think the
way to do that is to be very careful about what type of overloading we
allow.


> 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.
>
>
How about we try going a little bit deeper here and we assume that the* *name
of the module is accounted for by the class loader.  How about these rules:

1) Put all code into modules/[MODULE]/classes/[TYPE].  eg, tag_event =>
modules/tag/classes/Event.php containing "class Tag_Event"

2) To access a class like X_Y, the autoloader looks in
modules/X/classes/Y.php which allows us to incorporate the class name.

3) The class search order for X_Y_Z would be:
modules/X/classes/Y/Z
modules/classes/X/Y/Z

This allows the following:
- our directory structure is very flat, there's only one place for the code
- each module can provide its own implementation, for example you'd write:

    modules/gallery/classes/Controller/Albums.php:
    class Gallery_Controller_Albums {}

  then if you want to overload it in the tag module you'd do:

    modules/tag/classes/Controller/Albums.php:
    class Tag_Controller_Albums extends Gallery_Controller_Albums {}

  and when you do "new Controller_Albums" it'll pull Tag_Controller_Albums
first, which will trigger the equivalent of "new Gallery_Controller_Albums"
which will pull the rest.

Pros
- A lot less files!
- Easier for devs to understand
- I *think* that it's backwards compatible with the existing Kohana modules
that we've already integrated

Cons:
- it's us, doing our own thing, again.  sigh
- we have to extend the class loader.  Is that hard in this case?  dunno


thoughts?
-Bharat

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