Re: Capitalization

On Mon, Mar 18, 2013 at 3:54 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:

> Thanks for the great reply, too!  You make some good points, and I should
> probably extract myself from K3-land and sleep on it before figuring out an
> equally-good reply, but I suspect the overall tone will be this: while I
> certainly agree that having fewer (relatively meaningless) files and a
> simpler structure for devs to understand is nice, I worry that this could
> be a slippery slope.  For example, why not just reimplement K2's autoloader
> and get total backward compatibility?
>
Good point - let's do it!  Haha, kidding.  I want to be *similar* to Kohana
3, but it doesn't have to be exact.  I think that it's a minor and
human-understandable change to consider the module name as part of the auto
load path.

> One off-the-cuff, not-totally-thought-through idea: if the major issue is
> (understandably) the doubling of files and the noise it brings to our repo
> and to devs trying to get one feature made, what if we just implemented
> that in the autoloader?  This is similar to what K2 did (used eval to
> extend the _Core, etc).  We could keep the K3-compatible structure and
> naming, but replace the empty files with just enough magic to keep
> everything still looking more-or-less like K3.
>
We could, but then we'd still have the problem that we're adding an extra
level of depth. For example, then all of the tag classes would be in:

  modules/tag/classes/Tag/Event.php
  modules/tag/classes/Tag/Installer.php
  modules/tag/classes/Tag/Task.php
  ...

modules/tag/classes would be largely empty, but the classes/Tag subdir has
to exist so that Tag_Event is consistent, and then our magic autoloader
would be creating modules/tag/classes/Event.php to extend Tag_Event for us.
 That magic would be even more confusing since there's no obvious way to
identify that it's happening.  Whereas with what I'm proposing it'd be:

  modules/tag/classes/Event.php
  modules/tag/classes/Installer.php
  modules/tag/classes/Task.php
  ...

And if you're looking for Tag_Event then you're looking in
modules/tag/classes/Event which is a little bit magic, but is also a fairly
simple rule.

My solution is definitely unorthodox.  But I'm not convinced that the extra
file-level complexity is worth the code duplication.  Who knows, we may be
able to convince Woody to take it upstream...


> Take care,
> Shad
>
> Sent from my Swiss Army Phone
> On Mar 18, 2013 11:28 PM, "Bharat Mediratta" <[email protected]> wrote:
>
>>
>> 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 ]